8288130: compiler error with AP and explicit record accessor

Reviewed-by: jlahoda

Author: Vicente Romero

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

@@ -1503,30 +1503,25 @@ public RecordComponent getRecordComponent(VarSymbol field) {
             return null;
         }
 
-        public RecordComponent getRecordComponent(JCVariableDecl var, boolean addIfMissing, List<JCAnnotation> 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<JCAnnotation> 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 RecordComponent(Name name, Type type, Symbol owner) {
         }
 
         public RecordComponent(VarSymbol field, List<JCAnnotation> annotations) {
+            this(field, annotations, field.type.hasTag(TypeTag.ARRAY) && ((ArrayType)field.type).isVarargs());
+        }
+
+        public RecordComponent(VarSymbol field, List<JCAnnotation> annotations, boolean isVarargs) {
             super(PUBLIC, field.name, field.type, field.owner);
             this.originalAnnos = annotations;
             this.pos = field.pos;
@@ -1817,7 +1816,7 @@ public RecordComponent(VarSymbol field, List<JCAnnotation> annotations) {
              * 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<JCAnnotation> getOriginalAnnos() { return originalAnnos; }

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

@@ -2960,6 +2960,9 @@ public void validateTypeAnnotations(List<JCAnnotation> annotations, boolean isTy
     /** 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());
 

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

@@ -983,7 +983,7 @@ protected void runPhase(Env<AttrContext> env) {
                 List<JCVariableDecl> 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<JCTree>(make.at(field.pos)).copy(field.mods.annotations));
@@ -1229,10 +1229,37 @@ private void addRecordMembersIfNeeded(JCClassDecl tree, Env<AttrContext> env) {
                 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<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

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java

@@ -65,8 +65,8 @@ public <T extends JCTree> List<T> copy(List<T> trees) {
     }
 
     public <T extends JCTree> List<T> copy(List<T> trees, P p) {
-        if (trees == null)
-            return null;
+        if (trees == null || trees.isEmpty())
+            return trees;
         ListBuffer<T> lb = new ListBuffer<>();
         for (T tree: trees)
             lb.append(copy(tree, p));