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

[Vicente Romero]

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
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20161205/38338233/attachment.html>