8332600: javac uses record components source position during compilation

Reviewed-by: jlahoda

Author: Vicente Romero

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

@@ -1564,29 +1564,21 @@ public RecordComponent getRecordComponent(VarSymbol field) {
             return null;
         }
 
-        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
-                 * case we need to use the position to disambiguate, but if we loaded the record from a class file
-                 * all positions will be -1, in that case we have to ignore the position and match only based on the
-                 * name
-                 */
-                if (rc.name == var.name && (var.pos == rc.pos || rc.pos == -1)) {
-                    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 rcDecl, VarSymbol varSym) {
             RecordComponent rc = null;
-            if (existing != null) {
-                recordComponents = List.filter(recordComponents, existing);
-                recordComponents = recordComponents.append(rc = new RecordComponent(varSym, existing.ast, existing.isVarargs));
+            if (existing != null && !recordComponents.isEmpty()) {
+                ListBuffer<RecordComponent> newRComps = new ListBuffer<>();
+                for (RecordComponent rcomp : recordComponents) {
+                    if (existing == rcomp) {
+                        newRComps.add(rc = new RecordComponent(varSym, existing.ast, existing.isVarargs));
+                    } else {
+                        newRComps.add(rcomp);
+                    }
+                }
+                recordComponents = newRComps.toList();
             } else {
                 // Didn't find the record component: create one.
                 recordComponents = recordComponents.append(rc = new RecordComponent(varSym, rcDecl));

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

@@ -1048,6 +1048,7 @@ protected void runPhase(Env<AttrContext> env) {
             if ((sym.flags_field & RECORD) != 0) {
                 List<JCVariableDecl> fields = TreeInfo.recordFields(tree);
 
+                int fieldPos = 0;
                 for (JCVariableDecl field : fields) {
                     /** 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,
@@ -1063,24 +1064,24 @@ protected void runPhase(Env<AttrContext> env) {
                      *  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
+                     *  First, we get the record component matching the field position. 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);
+                    RecordComponent rc = getRecordComponentAt(sym, fieldPos);
 
                     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;
+                        field.mods.annotations = tc.copy(rc.getOriginalAnnos());
                     }
 
                     memberEnter.memberEnter(field, env);
 
                     JCVariableDecl rcDecl = new TreeCopier<JCTree>(make.at(field.pos)).copy(field);
                     sym.createRecordComponent(rc, rcDecl, field.sym);
+                    fieldPos++;
                 }
 
                 enterThisAndSuper(sym, env);
@@ -1095,6 +1096,18 @@ protected void runPhase(Env<AttrContext> env) {
         }
     }
 
+    // where
+        private RecordComponent getRecordComponentAt(ClassSymbol sym, int componentPos) {
+            int i = 0;
+            for (RecordComponent rc : sym.getRecordComponents()) {
+                if (i == componentPos) {
+                    return rc;
+                }
+                i++;
+            }
+            return null;
+        }
+
     /** Enter member fields and methods of a class
      */
     private final class MembersPhase extends AbstractMembersPhase {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2024, Oracle and/or its affiliates. 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
@@ -26,6 +26,7 @@
  *
  * @test
  * @bug 8250629 8252307 8247352 8241151 8246774 8259025 8288130 8282714 8289647 8294020
+ *      8332600
  * @summary Negative compilation tests, and positive compilation (smoke) tests for records
  * @library /lib/combo /tools/lib /tools/javac/lib
  * @enablePreview
@@ -136,6 +137,7 @@ void testMalformedDeclarations() {
         assertFail("compiler.err.repeated.modifier", "public public record R(String foo) { }");
         assertFail("compiler.err.repeated.modifier", "private private record R(String foo) { }");
         assertFail("compiler.err.already.defined", "record R(int x, int x) {}");
+        assertFail("compiler.err.already.defined", "record R(int x, int x, int x) {}");
         for (String s : List.of("var", "record"))
             assertFail("compiler.err.restricted.type.not.allowed.here", "record R(# x) { }", s);
         for (String s : List.of("public", "protected", "private", "static", "final", "transient", "volatile",