diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java index 558d2b3107c..1d2dcb4ee18 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java @@ -1503,30 +1503,25 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem return null; } - public RecordComponent getRecordComponent(JCVariableDecl var, boolean addIfMissing, List annotations) { + /* creates a record component if non is related to the given variable and recreates a brand new one + * in other case + */ + public RecordComponent createRecordComponent(JCVariableDecl var, List annotations) { RecordComponent toRemove = null; for (RecordComponent rc : recordComponents) { /* it could be that a record erroneously declares two record components with the same name, in that * case we need to use the position to disambiguate */ if (rc.name == var.name && var.pos == rc.pos) { - if (rc.type.hasTag(TypeTag.ERROR) && !var.sym.type.hasTag(TypeTag.ERROR)) { - // Found a record component with an erroneous type: save it so that it can be removed later. - // If the class type of the record component is generated by annotation processor, it should - // use the new actual class type and symbol instead of the old dummy ErrorType. - toRemove = rc; - } else { - // Found a good record component: just return. - return rc; - } + toRemove = rc; } } RecordComponent rc = null; if (toRemove != null) { // Found a record component with an erroneous type: remove it and create a new one recordComponents = List.filter(recordComponents, toRemove); - recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, annotations)); - } else if (addIfMissing) { + recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, toRemove.originalAnnos, toRemove.isVarargs)); + } else { // Didn't find the record component: create one. recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, annotations)); } @@ -1809,6 +1804,10 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem } public RecordComponent(VarSymbol field, List annotations) { + this(field, annotations, field.type.hasTag(TypeTag.ARRAY) && ((ArrayType)field.type).isVarargs()); + } + + public RecordComponent(VarSymbol field, List annotations, boolean isVarargs) { super(PUBLIC, field.name, field.type, field.owner); this.originalAnnos = annotations; this.pos = field.pos; @@ -1817,7 +1816,7 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem * the symbol will be blown out and we won't be able to know if the original * record component was declared varargs or not. */ - this.isVarargs = type.hasTag(TypeTag.ARRAY) && ((ArrayType)type).isVarargs(); + this.isVarargs = isVarargs; } public List getOriginalAnnos() { return originalAnnos; } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 02a325cb717..5be4ad810d2 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -2960,6 +2960,9 @@ public class Check { /** Check an annotation of a symbol. */ private void validateAnnotation(JCAnnotation a, JCTree declarationTree, Symbol s) { + /** NOTE: if annotation processors are present, annotation processing rounds can happen after this method, + * this can impact in particular records for which annotations are forcibly propagated. + */ validateAnnotationTree(a); boolean isRecordMember = ((s.flags_field & RECORD) != 0 || s.enclClass() != null && s.enclClass().isRecord()); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java index f266524baa9..ce8ab32607b 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java @@ -983,7 +983,7 @@ public class TypeEnter implements Completer { List fields = TreeInfo.recordFields(tree); memberEnter.memberEnter(fields, env); for (JCVariableDecl field : fields) { - sym.getRecordComponent(field, true, + sym.createRecordComponent(field, field.mods.annotations.isEmpty() ? List.nil() : new TreeCopier(make.at(field.pos)).copy(field.mods.annotations)); @@ -1229,10 +1229,37 @@ public class TypeEnter implements Completer { memberEnter.memberEnter(equals, env); } - // fields can't be varargs, lets remove the flag + /** Some notes regarding the code below. Annotations applied to elements of a record header are propagated + * to other elements which, when applicable, not explicitly declared by the user: the canonical constructor, + * accessors, fields and record components. Of all these the only ones that can't be explicitly declared are + * the fields and the record components. + * + * Now given that annotations are propagated to all possible targets regardless of applicability, + * annotations not applicable to a given element should be removed. See Check::validateAnnotation. Once + * annotations are removed we could lose the whole picture, that's why original annotations are stored in + * the record component, see RecordComponent::originalAnnos, but there is no real AST representing a record + * component so if there is an annotation processing round it could be that we need to reenter a record for + * which we need to re-attribute its annotations. This is why one of the things the code below is doing is + * copying the original annotations from the record component to the corresponding field, again this applies + * only if APs are present. + * + * We need to copy the annotations to the field so that annotations applicable only to the record component + * can be attributed as if declared in the field and then stored in the metadata associated to the record + * component. + */ List recordFields = TreeInfo.recordFields(tree); for (JCVariableDecl field: recordFields) { + RecordComponent rec = tree.sym.getRecordComponent(field.sym); + TreeCopier tc = new TreeCopier<>(make.at(field.pos)); + List originalAnnos = tc.copy(rec.getOriginalAnnos()); + field.mods.flags &= ~Flags.VARARGS; + if (originalAnnos.length() != field.mods.annotations.length()) { + field.mods.annotations = originalAnnos; + annotate.annotateLater(originalAnnos, env, field.sym, field.pos()); + } + + // also here field.sym.flags_field &= ~Flags.VARARGS; } // now lets add the accessors diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java index 45590119b3c..90f7a8c4f1e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java @@ -65,8 +65,8 @@ public class TreeCopier

implements TreeVisitor { } public List copy(List trees, P p) { - if (trees == null) - return null; + if (trees == null || trees.isEmpty()) + return trees; ListBuffer lb = new ListBuffer<>(); for (T tree: trees) lb.append(copy(tree, p)); diff --git a/test/langtools/tools/javac/records/RecordCompilationTests.java b/test/langtools/tools/javac/records/RecordCompilationTests.java index ca53ec63e39..c112aabdf27 100644 --- a/test/langtools/tools/javac/records/RecordCompilationTests.java +++ b/test/langtools/tools/javac/records/RecordCompilationTests.java @@ -25,7 +25,7 @@ * RecordCompilationTests * * @test - * @bug 8250629 8252307 8247352 8241151 8246774 8259025 + * @bug 8250629 8252307 8247352 8241151 8246774 8259025 8288130 * @summary Negative compilation tests, and positive compilation (smoke) tests for records * @library /lib/combo /tools/lib /tools/javac/lib * @modules @@ -177,6 +177,23 @@ public class RecordCompilationTests extends CompilationTestCase { assertOK("@Deprecated record R(int x, int y) { }"); assertOK("record R(@Deprecated int x, int y) { }"); assertOK("record R(T x, T y) { }"); + assertOK( + """ + record R(T x) { + public T x() { + return this.x; + } + } + """); + assertOK( + """ + import java.util.List; + record R(List x) { + public List x() { + return this.x; + } + } + """); } public void testGoodMemberDeclarations() { @@ -1261,9 +1278,8 @@ public class RecordCompilationTests extends CompilationTestCase { Assert.check(numberOfFieldRefs == 1); } - /* check that fields are initialized in a canonical constructor in the same declaration order as the corresponding - * record component - */ + // check that fields are initialized in a canonical constructor in the same declaration order as the corresponding + // record component public void testCheckInitializationOrderInCompactConstructor() throws Exception { int putField1 = -1; int putField2 = -1; @@ -1304,179 +1320,180 @@ public class RecordCompilationTests extends CompilationTestCase { public void testAcceptRecordId() { String[] previousOptions = getCompileOptions(); - String[] testOptions = {/* no options */}; - setCompileOptions(testOptions); - assertFail("compiler.err.illegal.start.of.type", - "class R {\n" + - " record RR(int i) {\n" + - " return null;\n" + - " }\n" + - " class record {}\n" + - "}"); - setCompileOptions(previousOptions); + try { + String[] testOptions = {}; + setCompileOptions(testOptions); + assertFail("compiler.err.illegal.start.of.type", + "class R {\n" + + " record RR(int i) {\n" + + " return null;\n" + + " }\n" + + " class record {}\n" + + "}"); + } finally { + setCompileOptions(previousOptions); + } } public void testAnnos() throws Exception { String[] previousOptions = getCompileOptions(); - String srcTemplate = - """ + try { + String srcTemplate = + """ import java.lang.annotation.*; @Target({#TARGET}) @Retention(RetentionPolicy.RUNTIME) @interface Anno { } - record R(@Anno String s) {} - """; + """; - // testing several combinations, adding even more combinations won't add too much value - List annoApplicableTargets = List.of( - "ElementType.FIELD", - "ElementType.METHOD", - "ElementType.PARAMETER", - "ElementType.RECORD_COMPONENT", - "ElementType.TYPE_USE", - "ElementType.TYPE_USE,ElementType.FIELD", - "ElementType.TYPE_USE,ElementType.METHOD", - "ElementType.TYPE_USE,ElementType.PARAMETER", - "ElementType.TYPE_USE,ElementType.RECORD_COMPONENT", - "ElementType.TYPE_USE,ElementType.FIELD,ElementType.METHOD", - "ElementType.TYPE_USE,ElementType.FIELD,ElementType.PARAMETER", - "ElementType.TYPE_USE,ElementType.FIELD,ElementType.RECORD_COMPONENT", - "ElementType.FIELD,ElementType.TYPE_USE", - "ElementType.FIELD,ElementType.CONSTRUCTOR", - "ElementType.FIELD,ElementType.LOCAL_VARIABLE", - "ElementType.FIELD,ElementType.ANNOTATION_TYPE", - "ElementType.FIELD,ElementType.PACKAGE", - "ElementType.FIELD,ElementType.TYPE_PARAMETER", - "ElementType.FIELD,ElementType.MODULE", - "ElementType.METHOD,ElementType.TYPE_USE", - "ElementType.PARAMETER,ElementType.TYPE_USE", - "ElementType.RECORD_COMPONENT,ElementType.TYPE_USE", - "ElementType.FIELD,ElementType.METHOD,ElementType.TYPE_USE", - "ElementType.FIELD,ElementType.PARAMETER,ElementType.TYPE_USE", - "ElementType.FIELD,ElementType.RECORD_COMPONENT,ElementType.TYPE_USE" - ); + // testing several combinations, adding even more combinations won't add too much value + List annoApplicableTargets = List.of( + "ElementType.FIELD", + "ElementType.METHOD", + "ElementType.PARAMETER", + "ElementType.RECORD_COMPONENT", + "ElementType.TYPE_USE", + "ElementType.TYPE_USE,ElementType.FIELD", + "ElementType.TYPE_USE,ElementType.METHOD", + "ElementType.TYPE_USE,ElementType.PARAMETER", + "ElementType.TYPE_USE,ElementType.RECORD_COMPONENT", + "ElementType.TYPE_USE,ElementType.FIELD,ElementType.METHOD", + "ElementType.TYPE_USE,ElementType.FIELD,ElementType.PARAMETER", + "ElementType.TYPE_USE,ElementType.FIELD,ElementType.RECORD_COMPONENT", + "ElementType.FIELD,ElementType.TYPE_USE", + "ElementType.FIELD,ElementType.CONSTRUCTOR", + "ElementType.FIELD,ElementType.LOCAL_VARIABLE", + "ElementType.FIELD,ElementType.ANNOTATION_TYPE", + "ElementType.FIELD,ElementType.PACKAGE", + "ElementType.FIELD,ElementType.TYPE_PARAMETER", + "ElementType.FIELD,ElementType.MODULE", + "ElementType.METHOD,ElementType.TYPE_USE", + "ElementType.PARAMETER,ElementType.TYPE_USE", + "ElementType.RECORD_COMPONENT,ElementType.TYPE_USE", + "ElementType.FIELD,ElementType.METHOD,ElementType.TYPE_USE", + "ElementType.FIELD,ElementType.PARAMETER,ElementType.TYPE_USE", + "ElementType.FIELD,ElementType.RECORD_COMPONENT,ElementType.TYPE_USE" + ); - String[] generalOptions = { - "-processor", Processor.class.getName(), - "-Atargets=" - }; + String[] generalOptions = { + "-processor", Processor.class.getName(), + "-Atargets=" + }; - for (String target : annoApplicableTargets) { - String code = srcTemplate.replaceFirst("#TARGET", target); - String[] testOptions = generalOptions.clone(); - testOptions[testOptions.length - 1] = testOptions[testOptions.length - 1] + target; - setCompileOptions(testOptions); + for (String target : annoApplicableTargets) { + String code = srcTemplate.replaceFirst("#TARGET", target); + String[] testOptions = generalOptions.clone(); + testOptions[testOptions.length - 1] = testOptions[testOptions.length - 1] + target; + setCompileOptions(testOptions); - File dir = assertOK(true, code); + File dir = assertOK(true, code); - ClassFile classFile = ClassFile.read(findClassFileOrFail(dir, "R.class")); + ClassFile classFile = ClassFile.read(findClassFileOrFail(dir, "R.class")); - // field first - Assert.check(classFile.fields.length == 1); - Field field = classFile.fields[0]; - /* if FIELD is one of the targets then there must be a declaration annotation applied to the field, apart from - * the type annotation - */ - if (target.contains("ElementType.FIELD")) { - checkAnno(classFile, - (RuntimeAnnotations_attribute)findAttributeOrFail( - field.attributes, - RuntimeVisibleAnnotations_attribute.class), - "Anno"); - } else { - assertAttributeNotPresent(field.attributes, RuntimeVisibleAnnotations_attribute.class); + // field first + Assert.check(classFile.fields.length == 1); + Field field = classFile.fields[0]; + // if FIELD is one of the targets then there must be a declaration annotation applied to the field, apart from + // the type annotation + if (target.contains("ElementType.FIELD")) { + checkAnno(classFile, + (RuntimeAnnotations_attribute) findAttributeOrFail( + field.attributes, + RuntimeVisibleAnnotations_attribute.class), + "Anno"); + } else { + assertAttributeNotPresent(field.attributes, RuntimeVisibleAnnotations_attribute.class); + } + + // lets check now for the type annotation + if (target.contains("ElementType.TYPE_USE")) { + checkTypeAnno( + classFile, + (RuntimeVisibleTypeAnnotations_attribute) findAttributeOrFail(field.attributes, RuntimeVisibleTypeAnnotations_attribute.class), + "FIELD", + "Anno"); + } else { + assertAttributeNotPresent(field.attributes, RuntimeVisibleTypeAnnotations_attribute.class); + } + + // checking for the annotation on the corresponding parameter of the canonical constructor + Method init = findMethodOrFail(classFile, ""); + // if PARAMETER is one of the targets then there must be a declaration annotation applied to the parameter, apart from + // the type annotation + if (target.contains("ElementType.PARAMETER")) { + checkParameterAnno(classFile, + (RuntimeVisibleParameterAnnotations_attribute) findAttributeOrFail( + init.attributes, + RuntimeVisibleParameterAnnotations_attribute.class), + "Anno"); + } else { + assertAttributeNotPresent(init.attributes, RuntimeVisibleAnnotations_attribute.class); + } + // let's check now for the type annotation + if (target.contains("ElementType.TYPE_USE")) { + checkTypeAnno( + classFile, + (RuntimeVisibleTypeAnnotations_attribute) findAttributeOrFail(init.attributes, RuntimeVisibleTypeAnnotations_attribute.class), + "METHOD_FORMAL_PARAMETER", "Anno"); + } else { + assertAttributeNotPresent(init.attributes, RuntimeVisibleTypeAnnotations_attribute.class); + } + + // checking for the annotation in the accessor + Method accessor = findMethodOrFail(classFile, "s"); + // if METHOD is one of the targets then there must be a declaration annotation applied to the accessor, apart from + // the type annotation + if (target.contains("ElementType.METHOD")) { + checkAnno(classFile, + (RuntimeAnnotations_attribute) findAttributeOrFail( + accessor.attributes, + RuntimeVisibleAnnotations_attribute.class), + "Anno"); + } else { + assertAttributeNotPresent(accessor.attributes, RuntimeVisibleAnnotations_attribute.class); + } + // let's check now for the type annotation + if (target.contains("ElementType.TYPE_USE")) { + checkTypeAnno( + classFile, + (RuntimeVisibleTypeAnnotations_attribute) findAttributeOrFail(accessor.attributes, RuntimeVisibleTypeAnnotations_attribute.class), + "METHOD_RETURN", "Anno"); + } else { + assertAttributeNotPresent(accessor.attributes, RuntimeVisibleTypeAnnotations_attribute.class); + } + + // checking for the annotation in the Record attribute + Record_attribute record = (Record_attribute) findAttributeOrFail(classFile.attributes, Record_attribute.class); + Assert.check(record.component_count == 1); + // if RECORD_COMPONENT is one of the targets then there must be a declaration annotation applied to the + // field, apart from the type annotation + if (target.contains("ElementType.RECORD_COMPONENT")) { + checkAnno(classFile, + (RuntimeAnnotations_attribute) findAttributeOrFail( + record.component_info_arr[0].attributes, + RuntimeVisibleAnnotations_attribute.class), + "Anno"); + } else { + assertAttributeNotPresent(record.component_info_arr[0].attributes, RuntimeVisibleAnnotations_attribute.class); + } + // lets check now for the type annotation + if (target.contains("ElementType.TYPE_USE")) { + checkTypeAnno( + classFile, + (RuntimeVisibleTypeAnnotations_attribute) findAttributeOrFail( + record.component_info_arr[0].attributes, + RuntimeVisibleTypeAnnotations_attribute.class), + "FIELD", "Anno"); + } else { + assertAttributeNotPresent(record.component_info_arr[0].attributes, RuntimeVisibleTypeAnnotations_attribute.class); + } } - // lets check now for the type annotation - if (target.contains("ElementType.TYPE_USE")) { - checkTypeAnno( - classFile, - (RuntimeVisibleTypeAnnotations_attribute)findAttributeOrFail(field.attributes, RuntimeVisibleTypeAnnotations_attribute.class), - "FIELD", - "Anno"); - } else { - assertAttributeNotPresent(field.attributes, RuntimeVisibleTypeAnnotations_attribute.class); - } - - // checking for the annotation on the corresponding parameter of the canonical constructor - Method init = findMethodOrFail(classFile, ""); - /* if PARAMETER is one of the targets then there must be a declaration annotation applied to the parameter, apart from - * the type annotation - */ - if (target.contains("ElementType.PARAMETER")) { - checkParameterAnno(classFile, - (RuntimeVisibleParameterAnnotations_attribute)findAttributeOrFail( - init.attributes, - RuntimeVisibleParameterAnnotations_attribute.class), - "Anno"); - } else { - assertAttributeNotPresent(init.attributes, RuntimeVisibleAnnotations_attribute.class); - } - // let's check now for the type annotation - if (target.contains("ElementType.TYPE_USE")) { - checkTypeAnno( - classFile, - (RuntimeVisibleTypeAnnotations_attribute) findAttributeOrFail(init.attributes, RuntimeVisibleTypeAnnotations_attribute.class), - "METHOD_FORMAL_PARAMETER", "Anno"); - } else { - assertAttributeNotPresent(init.attributes, RuntimeVisibleTypeAnnotations_attribute.class); - } - - // checking for the annotation in the accessor - Method accessor = findMethodOrFail(classFile, "s"); - /* if METHOD is one of the targets then there must be a declaration annotation applied to the accessor, apart from - * the type annotation - */ - if (target.contains("ElementType.METHOD")) { - checkAnno(classFile, - (RuntimeAnnotations_attribute)findAttributeOrFail( - accessor.attributes, - RuntimeVisibleAnnotations_attribute.class), - "Anno"); - } else { - assertAttributeNotPresent(accessor.attributes, RuntimeVisibleAnnotations_attribute.class); - } - // let's check now for the type annotation - if (target.contains("ElementType.TYPE_USE")) { - checkTypeAnno( - classFile, - (RuntimeVisibleTypeAnnotations_attribute)findAttributeOrFail(accessor.attributes, RuntimeVisibleTypeAnnotations_attribute.class), - "METHOD_RETURN", "Anno"); - } else { - assertAttributeNotPresent(accessor.attributes, RuntimeVisibleTypeAnnotations_attribute.class); - } - - // checking for the annotation in the Record attribute - Record_attribute record = (Record_attribute)findAttributeOrFail(classFile.attributes, Record_attribute.class); - Assert.check(record.component_count == 1); - /* if RECORD_COMPONENT is one of the targets then there must be a declaration annotation applied to the - * field, apart from the type annotation - */ - if (target.contains("ElementType.RECORD_COMPONENT")) { - checkAnno(classFile, - (RuntimeAnnotations_attribute)findAttributeOrFail( - record.component_info_arr[0].attributes, - RuntimeVisibleAnnotations_attribute.class), - "Anno"); - } else { - assertAttributeNotPresent(record.component_info_arr[0].attributes, RuntimeVisibleAnnotations_attribute.class); - } - // lets check now for the type annotation - if (target.contains("ElementType.TYPE_USE")) { - checkTypeAnno( - classFile, - (RuntimeVisibleTypeAnnotations_attribute)findAttributeOrFail( - record.component_info_arr[0].attributes, - RuntimeVisibleTypeAnnotations_attribute.class), - "FIELD", "Anno"); - } else { - assertAttributeNotPresent(record.component_info_arr[0].attributes, RuntimeVisibleTypeAnnotations_attribute.class); - } + // let's reset the default compiler options for other tests + } finally { + setCompileOptions(previousOptions); } - - // let's reset the default compiler options for other tests - setCompileOptions(previousOptions); } private void checkTypeAnno(ClassFile classFile, @@ -1655,7 +1672,7 @@ public class RecordCompilationTests extends CompilationTestCase { switch (method.getName(classFile.constant_pool)) { case "toString", "equals", "hashCode" -> Assert.check(method.access_flags.is(AccessFlags.ACC_PUBLIC) && method.access_flags.is(AccessFlags.ACC_FINAL)); - default -> { /* do nothing */ } + default -> {} } } } @@ -1963,9 +1980,8 @@ public class RecordCompilationTests extends CompilationTestCase { public void testNoWarningForSerializableRecords() { if (!useAP) { - /* dont execute this test when the default annotation processor is on as it will fail due to - * spurious warnings - */ + // dont execute this test when the default annotation processor is on as it will fail due to + // spurious warnings appendCompileOptions("-Werror", "-Xlint:serial"); assertOK( """