RFR 8170667: ClassReader assigns method parameters from MethodParameters incorrectly when long/double parameters are present

[Jan Lahoda]

Thanks Liam, Vicente!

Pushed:
http://hg.openjdk.java.net/jdk9/dev/langtools/rev/5d43af61155b

Jan

On 7.12.2016 21:10, Liam Miller-Cushon wrote:
> Thanks for the review! I moved the entire test into one file, and fixed
> the style issues.
>
> On Mon, Dec 5, 2016 at 3:36 PM, Vicente Romero
> <vicente.romero at oracle.com <mailto:vicente.romero at oracle.com>> wrote:
>
>     Hi Liam,
>
>     Thanks for the patch. It's probably a question of style but it seems
>     to me like the number of classes used could be reduced. For example,
>     the annotation processor could define the annotated methods that it
>     will later discover and process. This is just a suggestion. Also
>     regarding indentation, it's preferable to use 4 spaces instead of 2.
>
>     Thanks,
>     Vicente
>
>
>     On 12/02/2016 11:41 AM, Liam Miller-Cushon wrote:
>>     Thanks Jan, I've attached the full patch with a test.
>>
>>     On Fri, Dec 2, 2016 at 5:31 AM, Jan Lahoda <jan.lahoda at oracle.com
>>     <mailto:jan.lahoda at oracle.com>> wrote:
>>
>>         Hi Liam,
>>
>>         Thanks for the report and patch, I've filled:
>>         https://bugs.openjdk.java.net/browse/JDK-8170667
>>         <https://bugs.openjdk.java.net/browse/JDK-8170667>
>>
>>         I think you are right about the problem, and the fix seems
>>         very reasonable to me.
>>
>>         Jan
>>
>>
>>         On 2.12.2016 03:09, Liam Miller-Cushon wrote:
>>
>>             I think I found a MethodParameters class reading bug that
>>             affects
>>             annotation processing.
>>
>>             If MethodParameters attributes are not available parameter
>>             names are
>>             inferred from the LVT, which uses two slots for doubles
>>             and longs. The
>>             logic for skipping the extra slot is used even if the data
>>             was read from
>>             a MethodParameters attribute, where the indices are always
>>             contiguous.
>>             So if javac sees a parameter of type double or long it
>>             skips the next
>>             entry in the MethodParameters table.
>>
>>             There's a simple fix:
>>
>>             diff -r 775a385c4af6
>>             src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
>>             ---
>>             a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.javaThu
>>             Dec 01 17:25:45 2016 -0800
>>             +++
>>             b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.javaThu
>>             Dec 01 18:05:14 2016 -0800
>>             @@ -2321,7 +2321,7 @@
>>                                   ? parameterNameIndices[index] : 0);
>>                           Name name = nameIdx == 0 ? names.empty :
>>             readName(nameIdx);
>>                           paramNames = paramNames.prepend(name);
>>             -            index += Code.width(t);
>>             +            index += sawMethodParameters ? 1 : Code.width(t);
>>                       }
>>                       sym.savedParameterNames = paramNames.reverse();
>>                   }
>>
>>             If my diagnosis sounds correct, I can clean up the patch
>>             and add a test.
>>
>>             Here's the full repro:
>>
>>             === Lib.java
>>             class Lib {
>>                void f(int a, long b, int c, int d) {}
>>             }
>>             === Test.java
>>             @Deprecated
>>             class Test {}
>>             === ParameterProcessor.java
>>             import java.util.Set;
>>             import javax.annotation.processing.*;
>>             import javax.lang.model.SourceVersion;
>>             import javax.lang.model.element.*;
>>
>>             @SupportedAnnotationTypes("*")
>>             public class ParameterProcessor extends AbstractProcessor {
>>                @Override
>>                public boolean process(Set<? extends TypeElement>
>>             annotations,
>>             RoundEnvironment roundEnv) {
>>                  if (roundEnv.processingOver()) {
>>                    return false;
>>                  }
>>                  TypeElement s =
>>             processingEnv.getElementUtils().getTypeElement("Lib");
>>                  for (Element e : s.getEnclosedElements()) {
>>                    if (e instanceof ExecutableElement) {
>>                      for (VariableElement v : ((ExecutableElement)
>>             e).getParameters()) {
>>                        System.err.printf("  %s %s\n", v.asType(),
>>             v.getSimpleName());
>>                      }
>>                    }
>>                  }
>>                  return false;
>>                }
>>
>>                @Override
>>                public SourceVersion getSupportedSourceVersion() {
>>                  return SourceVersion.latest();
>>                }
>>             }
>>
>>             $ javac -parameters Lib.java
>>             $ javac ParameterProcessor.java
>>             $ javac -processor ParameterProcessor Test.java
>>                int a
>>                long b
>>                int d
>>                int arg3
>>
>>             Note that the third parameter is given the fourth
>>             parameter's name, and
>>             the fourth parameter's name is unknown.
>>
>>             Thanks,
>>
>>             Liam
>>
>>
>
>