# HG changeset patch # User cushon # Date 1513718665 18000 # Node ID 89f6aa26fd6c51b4c182172309142e1566d10045 # Parent d4329843abf42bb9415646155e696c3dca26de30 8007720: Names are not loaded correctly for method parameters if the parameters have annotations 8177486: Incorrect handling of mandated parameter names in MethodParameters attributes Reviewed-by: jlahoda, vromero diff -r d4329843abf4 -r 89f6aa26fd6c src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java Mon Dec 18 18:51:40 2017 -0800 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java Tue Dec 19 16:24:25 2017 -0500 @@ -1628,9 +1628,6 @@ /** The parameters of the method. */ public List params = null; - /** The names of the parameters */ - public List savedParameterNames; - /** For an annotation type element, its default value if any. * The value is null if none appeared in the method * declaration. @@ -1886,59 +1883,20 @@ public List params() { owner.complete(); if (params == null) { - // If ClassReader.saveParameterNames has been set true, then - // savedParameterNames will be set to a list of names that - // matches the types in type.getParameterTypes(). If any names - // were not found in the class file, those names in the list will - // be set to the empty name. - // If ClassReader.saveParameterNames has been set false, then - // savedParameterNames will be null. - List paramNames = savedParameterNames; - savedParameterNames = null; - // discard the provided names if the list of names is the wrong size. - if (paramNames == null || paramNames.size() != type.getParameterTypes().size()) { - paramNames = List.nil(); - } - ListBuffer buf = new ListBuffer<>(); - List remaining = paramNames; - // assert: remaining and paramNames are both empty or both - // have same cardinality as type.getParameterTypes() + ListBuffer newParams = new ListBuffer<>(); int i = 0; for (Type t : type.getParameterTypes()) { - Name paramName; - if (remaining.isEmpty()) { - // no names for any parameters available - paramName = createArgName(i, paramNames); - } else { - paramName = remaining.head; - remaining = remaining.tail; - if (paramName.isEmpty()) { - // no name for this specific parameter - paramName = createArgName(i, paramNames); - } - } - buf.append(new VarSymbol(PARAMETER, paramName, t, this)); - i++; + Name paramName = name.table.fromString("arg" + i); + VarSymbol param = new VarSymbol(PARAMETER, paramName, t, this); + newParams.append(param); + } - params = buf.toList(); + params = newParams.toList(); } + Assert.checkNonNull(params); return params; } - // Create a name for the argument at position 'index' that is not in - // the exclude list. In normal use, either no names will have been - // provided, in which case the exclude list is empty, or all the names - // will have been provided, in which case this method will not be called. - private Name createArgName(int index, List exclude) { - String prefix = "arg"; - while (true) { - Name argName = name.table.fromString(prefix + index); - if (!exclude.contains(argName)) - return argName; - prefix += "$"; - } - } - public Symbol asMemberOf(Type site, Types types) { return new MethodSymbol(flags_field, name, types.memberType(site, this), owner); } diff -r d4329843abf4 -r 89f6aa26fd6c src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Mon Dec 18 18:51:40 2017 -0800 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Tue Dec 19 16:24:25 2017 -0500 @@ -193,6 +193,26 @@ int[] parameterNameIndices; /** + * A table to hold annotations for method parameters. + */ + ParameterAnnotations[] parameterAnnotations; + + /** + * A holder for parameter annotations. + */ + static class ParameterAnnotations { + List proxies; + + void add(List newAnnotations) { + if (proxies == null) { + proxies = newAnnotations; + } else { + proxies = proxies.prependList(newAnnotations); + } + } + } + + /** * Whether or not any parameter names have been found. */ boolean haveParameterNameIndices; @@ -1218,7 +1238,7 @@ new AttributeReader(names.RuntimeInvisibleParameterAnnotations, V49, CLASS_OR_MEMBER_ATTRIBUTE) { protected void read(Symbol sym, int attrLen) { - attachParameterAnnotations(sym); + readParameterAnnotations(sym); } }, @@ -1230,7 +1250,7 @@ new AttributeReader(names.RuntimeVisibleParameterAnnotations, V49, CLASS_OR_MEMBER_ATTRIBUTE) { protected void read(Symbol sym, int attrLen) { - attachParameterAnnotations(sym); + readParameterAnnotations(sym); } }, @@ -1288,10 +1308,14 @@ int numEntries = nextByte(); parameterNameIndices = new int[numEntries]; haveParameterNameIndices = true; + int index = 0; for (int i = 0; i < numEntries; i++) { int nameIndex = nextChar(); int flags = nextChar(); - parameterNameIndices[i] = nameIndex; + if ((flags & (Flags.MANDATED | Flags.SYNTHETIC)) != 0) { + continue; + } + parameterNameIndices[index++] = nameIndex; } } bp = newbp; @@ -1571,65 +1595,82 @@ * Reading Java-language annotations ***********************************************************************/ + /** + * Save annotations. + */ + List readAnnotations() { + int numAttributes = nextChar(); + ListBuffer annotations = new ListBuffer<>(); + for (int i = 0; i < numAttributes; i++) { + annotations.append(readCompoundAnnotation()); + } + return annotations.toList(); + } + /** Attach annotations. */ void attachAnnotations(final Symbol sym) { - int numAttributes = nextChar(); - if (numAttributes != 0) { - ListBuffer proxies = new ListBuffer<>(); - for (int i = 0; i v: proxy.values) { - if (v.fst == names.value && v.snd instanceof Attribute.Constant) { - Attribute.Constant c = (Attribute.Constant) v.snd; - if (c.type == syms.intType && ((Integer) c.value) > profile.value) { - sym.flags_field |= NOT_IN_PROFILE; - } + attachAnnotations(sym, readAnnotations()); + } + + /** + * Attach annotations. + */ + void attachAnnotations(final Symbol sym, List annotations) { + if (annotations.isEmpty()) { + return; + } + ListBuffer proxies = new ListBuffer<>(); + for (CompoundAnnotationProxy proxy : annotations) { + if (proxy.type.tsym == syms.proprietaryType.tsym) + sym.flags_field |= PROPRIETARY; + else if (proxy.type.tsym == syms.profileType.tsym) { + if (profile != Profile.DEFAULT) { + for (Pair v : proxy.values) { + if (v.fst == names.value && v.snd instanceof Attribute.Constant) { + Attribute.Constant c = (Attribute.Constant)v.snd; + if (c.type == syms.intType && ((Integer)c.value) > profile.value) { + sym.flags_field |= NOT_IN_PROFILE; } } } - } else { - if (proxy.type.tsym == syms.annotationTargetType.tsym) { - target = proxy; - } else if (proxy.type.tsym == syms.repeatableType.tsym) { - repeatable = proxy; - } else if (proxy.type.tsym == syms.deprecatedType.tsym) { - sym.flags_field |= (DEPRECATED | DEPRECATED_ANNOTATION); - for (Pair v : proxy.values) { - if (v.fst == names.forRemoval && v.snd instanceof Attribute.Constant) { - Attribute.Constant c = (Attribute.Constant) v.snd; - if (c.type == syms.booleanType && ((Integer) c.value) != 0) { - sym.flags_field |= DEPRECATED_REMOVAL; - } + } + } else { + if (proxy.type.tsym == syms.annotationTargetType.tsym) { + target = proxy; + } else if (proxy.type.tsym == syms.repeatableType.tsym) { + repeatable = proxy; + } else if (proxy.type.tsym == syms.deprecatedType.tsym) { + sym.flags_field |= (DEPRECATED | DEPRECATED_ANNOTATION); + for (Pair v : proxy.values) { + if (v.fst == names.forRemoval && v.snd instanceof Attribute.Constant) { + Attribute.Constant c = (Attribute.Constant)v.snd; + if (c.type == syms.booleanType && ((Integer)c.value) != 0) { + sym.flags_field |= DEPRECATED_REMOVAL; } } } - - proxies.append(proxy); } + proxies.append(proxy); } - annotate.normal(new AnnotationCompleter(sym, proxies.toList())); } + annotate.normal(new AnnotationCompleter(sym, proxies.toList())); } - /** Attach parameter annotations. + /** Read parameter annotations. */ - void attachParameterAnnotations(final Symbol method) { - final MethodSymbol meth = (MethodSymbol)method; + void readParameterAnnotations(Symbol meth) { int numParameters = buf[bp++] & 0xFF; - List parameters = meth.params(); - int pnum = 0; - while (parameters.tail != null) { - attachAnnotations(parameters.head); - parameters = parameters.tail; - pnum++; + if (parameterAnnotations == null) { + parameterAnnotations = new ParameterAnnotations[numParameters]; + } else if (parameterAnnotations.length != numParameters) { + throw badClassFile("bad.runtime.invisible.param.annotations", meth); } - if (pnum != numParameters) { - throw badClassFile("bad.runtime.invisible.param.annotations", meth); + for (int pnum = 0; pnum < numParameters; pnum++) { + if (parameterAnnotations[pnum] == null) { + parameterAnnotations[pnum] = new ParameterAnnotations(); + } + parameterAnnotations[pnum].add(readAnnotations()); } } @@ -2394,8 +2435,7 @@ } finally { currentOwner = prevOwner; } - if (saveParameterNames) - setParameterNames(m, type); + setParameters(m, type); if ((flags & VARARGS) != 0) { final Type last = type.getParameterTypes().last(); @@ -2448,22 +2488,17 @@ } /** - * Set the parameter names for a symbol from the name index in the - * parameterNameIndicies array. The type of the symbol may have changed - * while reading the method attributes (see the Signature attribute). - * This may be because of generic information or because anonymous - * synthetic parameters were added. The original type (as read from - * the method descriptor) is used to help guess the existence of + * Set the parameters for a method symbol, including any names and + * annotations that were read. + * + *

The type of the symbol may have changed while reading the + * method attributes (see the Signature attribute). This may be + * because of generic information or because anonymous synthetic + * parameters were added. The original type (as read from the + * method descriptor) is used to help guess the existence of * anonymous synthetic parameters. - * On completion, sym.savedParameter names will either be null (if - * no parameter names were found in the class file) or will be set to a - * list of names, one per entry in sym.type.getParameterTypes, with - * any missing names represented by the empty name. */ - void setParameterNames(MethodSymbol sym, Type jvmType) { - // if no names were found in the class file, there's nothing more to do - if (!haveParameterNameIndices) - return; + void setParameters(MethodSymbol sym, Type jvmType) { // If we get parameter names from MethodParameters, then we // don't need to skip. int firstParam = 0; @@ -2474,16 +2509,16 @@ // make a corresponding allowance here for the position of // the first parameter. Note that this assumes the // skipped parameter has a width of 1 -- i.e. it is not - // a double width type (long or double.) - if (sym.name == names.init && currentOwner.hasOuterInstance()) { - // Sometimes anonymous classes don't have an outer - // instance, however, there is no reliable way to tell so - // we never strip this$n - if (!currentOwner.name.isEmpty()) - firstParam += 1; - } + // a double width type (long or double.) + if (sym.name == names.init && currentOwner.hasOuterInstance()) { + // Sometimes anonymous classes don't have an outer + // instance, however, there is no reliable way to tell so + // we never strip this$n + if (!currentOwner.name.isEmpty()) + firstParam += 1; + } - if (sym.type != jvmType) { + if (sym.type != jvmType) { // reading the method attributes has caused the // symbol's type to be changed. (i.e. the Signature // attribute.) This may happen if there are hidden @@ -2493,21 +2528,55 @@ // at the beginning, and so skip over them. The // primary case for this is two hidden parameters // passed into Enum constructors. - int skip = Code.width(jvmType.getParameterTypes()) - - Code.width(sym.type.getParameterTypes()); - firstParam += skip; - } + int skip = Code.width(jvmType.getParameterTypes()) + - Code.width(sym.type.getParameterTypes()); + firstParam += skip; + } } List paramNames = List.nil(); - int index = firstParam; + ListBuffer params = new ListBuffer<>(); + int nameIndex = firstParam; + int annotationIndex = 0; for (Type t: sym.type.getParameterTypes()) { - int nameIdx = (index < parameterNameIndices.length - ? parameterNameIndices[index] : 0); - Name name = nameIdx == 0 ? names.empty : readName(nameIdx); + Name name = parameterName(nameIndex, paramNames); paramNames = paramNames.prepend(name); - index += sawMethodParameters ? 1 : Code.width(t); + VarSymbol param = new VarSymbol(PARAMETER, name, t, sym); + params.append(param); + if (parameterAnnotations != null) { + ParameterAnnotations annotations = parameterAnnotations[annotationIndex]; + if (annotations != null && annotations.proxies != null + && !annotations.proxies.isEmpty()) { + annotate.normal(new AnnotationCompleter(param, annotations.proxies)); + } + } + nameIndex += sawMethodParameters ? 1 : Code.width(t); + annotationIndex++; + } + if (parameterAnnotations != null && parameterAnnotations.length != annotationIndex) { + throw badClassFile("bad.runtime.invisible.param.annotations", sym); } - sym.savedParameterNames = paramNames.reverse(); + Assert.checkNull(sym.params); + sym.params = params.toList(); + parameterAnnotations = null; + parameterNameIndices = null; + } + + + // Returns the name for the parameter at position 'index', either using + // names read from the MethodParameters, or by synthesizing a name that + // is not on the 'exclude' list. + private Name parameterName(int index, List exclude) { + if (parameterNameIndices != null && index < parameterNameIndices.length + && parameterNameIndices[index] != 0) { + return readName(parameterNameIndices[index]); + } + String prefix = "arg"; + while (true) { + Name argName = names.fromString(prefix + exclude.size()); + if (!exclude.contains(argName)) + return argName; + prefix += "$"; + } } /** diff -r d4329843abf4 -r 89f6aa26fd6c test/langtools/tools/javac/MethodParameters/ClassReaderTest/ClassReaderTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/MethodParameters/ClassReaderTest/ClassReaderTest.java Tue Dec 19 16:24:25 2017 -0500 @@ -0,0 +1,73 @@ +/* + * Copyright 2017 Google Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8007720 8177486 + * @summary class reading of named parameters + * @library /tools/javac/lib + * @modules java.compiler + * jdk.compiler + * @compile -parameters ClassReaderTest.java MethodParameterProcessor.java + * @compile/process/ref=ClassReaderTest.out -proc:only -processor MethodParameterProcessor ClassReaderTest ClassReaderTest$I ClassReaderTest$E + */ + +import static java.lang.annotation.RetentionPolicy.CLASS; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; + +public class ClassReaderTest { + + @Retention(RUNTIME) + @interface RuntimeAnnoOne { + int value() default 0; + } + + @Retention(RUNTIME) + @interface RuntimeAnnoTwo { + int value() default 0; + } + + @Retention(CLASS) + @interface ClassAnno { + int value() default 0; + } + + @MethodParameterProcessor.ParameterNames + void f( + @RuntimeAnnoOne(1) @RuntimeAnnoTwo(2) @ClassAnno(3) int a, + @RuntimeAnnoOne(4) @RuntimeAnnoTwo(5) @ClassAnno(6) String b) {} + + class I { + @MethodParameterProcessor.ParameterNames + I(@ClassAnno(7) int d, @RuntimeAnnoOne(8) String e, Object o) {} + } + + enum E { + ONE(42, ""); + + @MethodParameterProcessor.ParameterNames + E(int x, @RuntimeAnnoOne(9) String s) {} + } +} diff -r d4329843abf4 -r 89f6aa26fd6c test/langtools/tools/javac/MethodParameters/ClassReaderTest/ClassReaderTest.out --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/MethodParameters/ClassReaderTest/ClassReaderTest.out Tue Dec 19 16:24:25 2017 -0500 @@ -0,0 +1,3 @@ +Note: ClassReaderTest.E.(x, @ClassReaderTest.RuntimeAnnoOne(9) s) +Note: ClassReaderTest.I.(@ClassReaderTest.ClassAnno(7) d, @ClassReaderTest.RuntimeAnnoOne(8) e, o) +Note: ClassReaderTest.f(@ClassReaderTest.ClassAnno(3) @ClassReaderTest.RuntimeAnnoOne(1) @ClassReaderTest.RuntimeAnnoTwo(2) a, @ClassReaderTest.ClassAnno(6) @ClassReaderTest.RuntimeAnnoOne(4) @ClassReaderTest.RuntimeAnnoTwo(5) b) diff -r d4329843abf4 -r 89f6aa26fd6c test/langtools/tools/javac/MethodParameters/ClassReaderTest/MethodParameterProcessor.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/langtools/tools/javac/MethodParameters/ClassReaderTest/MethodParameterProcessor.java Tue Dec 19 16:24:25 2017 -0500 @@ -0,0 +1,72 @@ +/* + * Copyright 2017 Google Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import static java.util.stream.Collectors.joining; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import javax.annotation.processing.RoundEnvironment; +import javax.annotation.processing.SupportedAnnotationTypes; +import javax.lang.model.element.Element; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.element.TypeElement; +import javax.tools.Diagnostic.Kind; + +@SupportedAnnotationTypes("MethodParameterProcessor.ParameterNames") +public class MethodParameterProcessor extends JavacTestingAbstractProcessor { + + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) + @interface ParameterNames { + String[] value() default {}; + } + + @Override + public boolean process(Set annotations, RoundEnvironment roundEnv) { + for (Element element : roundEnv.getElementsAnnotatedWith(ParameterNames.class)) { + ExecutableElement exec = (ExecutableElement)element; + String message = printNamesAndAnnotations(exec); + messager.printMessage(Kind.NOTE, message); + } + return false; + } + + private String printNamesAndAnnotations(ExecutableElement exec) { + return String.format("%s.%s(%s)", + exec.getEnclosingElement(), + exec.getSimpleName(), + exec.getParameters().stream().map(this::printParameter).collect(joining(", "))); + } + + private String printParameter(VariableElement param) { + return param.getAnnotationMirrors().stream().map(String::valueOf).collect(joining(" ")) + + (param.getAnnotationMirrors().isEmpty() ? "" : " ") + + param.getSimpleName(); + } +} diff -r d4329843abf4 -r 89f6aa26fd6c test/langtools/tools/javac/lib/DPrinter.java --- a/test/langtools/tools/javac/lib/DPrinter.java Mon Dec 18 18:51:40 2017 -0800 +++ b/test/langtools/tools/javac/lib/DPrinter.java Tue Dec 19 16:24:25 2017 -0500 @@ -1233,7 +1233,6 @@ public Void visitMethodSymbol(MethodSymbol sym, Void ignore) { // code printList("params", sym.params); - printList("savedParameterNames", sym.savedParameterNames); return visitSymbol(sym, null); }