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
--- 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<VarSymbol> params = null;
- /** The names of the parameters */
- public List<Name> 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<VarSymbol> 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<Name> 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<VarSymbol> buf = new ListBuffer<>();
- List<Name> remaining = paramNames;
- // assert: remaining and paramNames are both empty or both
- // have same cardinality as type.getParameterTypes()
+ ListBuffer<VarSymbol> 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<Name> 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);
}
--- 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<CompoundAnnotationProxy> proxies;
+
+ void add(List<CompoundAnnotationProxy> 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<CompoundAnnotationProxy> readAnnotations() {
+ int numAttributes = nextChar();
+ ListBuffer<CompoundAnnotationProxy> 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<CompoundAnnotationProxy> proxies = new ListBuffer<>();
- for (int i = 0; i<numAttributes; i++) {
- CompoundAnnotationProxy proxy = readCompoundAnnotation();
- 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<Name,Attribute> 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<CompoundAnnotationProxy> annotations) {
+ if (annotations.isEmpty()) {
+ return;
+ }
+ ListBuffer<CompoundAnnotationProxy> 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<Name, Attribute> 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<Name, Attribute> 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<Name, Attribute> 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<VarSymbol> 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.
+ *
+ * <p>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<Name> paramNames = List.nil();
- int index = firstParam;
+ ListBuffer<VarSymbol> 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<Name> 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 += "$";
+ }
}
/**
--- /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) {}
+ }
+}
--- /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.<init>(x, @ClassReaderTest.RuntimeAnnoOne(9) s)
+Note: ClassReaderTest.I.<init>(@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)
--- /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<? extends TypeElement> 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();
+ }
+}
--- 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);
}