8289647: AssertionError during annotation processing of record related tests

Reviewed-by: phh
Backport-of: 64a1a08ff9f120648e466449f65750991cbf673c

Author: AntonKozlov

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -1503,10 +1503,7 @@ public RecordComponent getRecordComponent(VarSymbol field) {
             return null;
         }
 
-        /* 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<JCAnnotation> annotations) {
+        public RecordComponent findRecordComponentToRemove(JCVariableDecl var) {
             RecordComponent toRemove = null;
             for (RecordComponent rc : recordComponents) {
                 /* it could be that a record erroneously declares two record components with the same name, in that
@@ -1516,11 +1513,17 @@ public RecordComponent createRecordComponent(JCVariableDecl var, List<JCAnnotati
                     toRemove = rc;
                 }
             }
+            return toRemove;
+        }
+
+        /* creates a record component if non is related to the given variable and recreates a brand new one
+         * in other case
+         */
+        public RecordComponent createRecordComponent(RecordComponent existing, JCVariableDecl var, List<JCAnnotation> annotations) {
             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, toRemove.originalAnnos, toRemove.isVarargs));
+            if (existing != null) {
+                recordComponents = List.filter(recordComponents, existing);
+                recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, existing.originalAnnos, existing.isVarargs));
             } else {
                 // Didn't find the record component: create one.
                 recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, annotations));

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java

@@ -963,9 +963,39 @@ protected void runPhase(Env<AttrContext> env) {
             ClassSymbol sym = tree.sym;
             if ((sym.flags_field & RECORD) != 0) {
                 List<JCVariableDecl> fields = TreeInfo.recordFields(tree);
-                memberEnter.memberEnter(fields, env);
+
                 for (JCVariableDecl field : fields) {
-                    sym.createRecordComponent(field,
+                    /** 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.
+                     *
+                     *  First, we find the record component by comparing its name and position with current field,
+                     *  if any, and we mark it. Then we 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. The invariance we need to keep here is that record components must be scheduled for
+                     *  annotation only once during this process.
+                     */
+                    RecordComponent rc = sym.findRecordComponentToRemove(field);
+
+                    if (rc != null && (rc.getOriginalAnnos().length() != field.mods.annotations.length())) {
+                        TreeCopier<JCTree> tc = new TreeCopier<>(make.at(field.pos));
+                        List<JCAnnotation> originalAnnos = tc.copy(rc.getOriginalAnnos());
+                        field.mods.annotations = originalAnnos;
+                    }
+
+                    memberEnter.memberEnter(field, env);
+
+                    sym.createRecordComponent(rc, field,
                             field.mods.annotations.isEmpty() ?
                                     List.nil() :
                                     new TreeCopier<JCTree>(make.at(field.pos)).copy(field.mods.annotations));
@@ -1211,37 +1241,10 @@ private void addRecordMembersIfNeeded(JCClassDecl tree, Env<AttrContext> env) {
                 memberEnter.memberEnter(equals, env);
             }
 
-            /** 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.
-             */
+            // fields can't be varargs, lets remove the flag
             List<JCVariableDecl> recordFields = TreeInfo.recordFields(tree);
             for (JCVariableDecl field: recordFields) {
-                RecordComponent rec = tree.sym.getRecordComponent(field.sym);
-                TreeCopier<JCTree> tc = new TreeCopier<>(make.at(field.pos));
-                List<JCAnnotation> 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

test/langtools/tools/javac/records/RecordCompilationTests.java

@@ -25,7 +25,7 @@
  * RecordCompilationTests
  *
  * @test
- * @bug 8250629 8252307 8247352 8241151 8246774 8259025 8288130
+ * @bug 8250629 8252307 8247352 8241151 8246774 8259025 8288130 8289647
  * @summary Negative compilation tests, and positive compilation (smoke) tests for records
  * @library /lib/combo /tools/lib /tools/javac/lib
  * @modules
@@ -1335,6 +1335,47 @@ public void testAcceptRecordId() {
         }
     }
 
+    public void testMultipleAnnosInRecord() throws Exception {
+        String[] previousOptions = getCompileOptions();
+
+        try {
+            String imports = """
+                    import java.lang.annotation.ElementType;
+                    import java.lang.annotation.Target;
+                    """;
+
+            String annotTemplate =
+                    """
+                    @Target(ElementType.#TARGET)
+                    @interface anno#TARGET { }
+                    """;
+
+            String recordTemplate =
+                    """
+                    record R(#TARGETS String s) {}
+                    """;
+
+            String[] generalOptions = {
+                    "-processor", Processor.class.getName(),
+            };
+
+            List<String> targets = List.of("FIELD", "RECORD_COMPONENT", "PARAMETER", "METHOD");
+
+            var interfaces = targets.stream().map(t -> annotTemplate.replaceAll("#TARGET", t)).collect(Collectors.joining("\n"));
+            var recordAnnotations = targets.stream().map(t -> "@anno" + t).collect(Collectors.joining(" "));
+            String record = recordTemplate.replaceFirst("#TARGETS", recordAnnotations);
+            String code = String.format("%s\n%s\n%s\n",imports,interfaces,record);
+            String[] testOptions = generalOptions.clone();
+            setCompileOptions(testOptions);
+
+            assertOK(true, code);
+
+        // let's reset the default compiler options for other tests
+        } finally {
+            setCompileOptions(previousOptions);
+        }
+    }
+
     public void testAnnos() throws Exception {
         String[] previousOptions = getCompileOptions();
         try {