From 245c08664896d63ac050ebc23259b23908dafed5 Mon Sep 17 00:00:00 2001 From: Vicente Romero <vromero@openjdk.org> Date: Thu, 18 Jul 2024 15:54:41 +0000 Subject: [PATCH] 8332600: javac uses record components source position during compilation Reviewed-by: jlahoda --- .../com/sun/tools/javac/code/Symbol.java | 28 +++++++------------ .../com/sun/tools/javac/comp/TypeEnter.java | 23 +++++++++++---- .../javac/records/RecordCompilationTests.java | 4 ++- 3 files changed, 31 insertions(+), 24 deletions(-) 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 4dd4f73a161..99f0645cd5c 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 @@ -1564,29 +1564,21 @@ public abstract class Symbol extends AnnoConstruct implements PoolConstant, Elem 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)); 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 eb48812f538..5be8d755d41 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 @@ -1048,6 +1048,7 @@ public class TypeEnter implements Completer { 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 @@ public class TypeEnter implements Completer { * 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 @@ public class TypeEnter implements Completer { } } + // 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 { diff --git a/test/langtools/tools/javac/records/RecordCompilationTests.java b/test/langtools/tools/javac/records/RecordCompilationTests.java index 9f5e73cc5d0..cbd00f9c607 100644 --- a/test/langtools/tools/javac/records/RecordCompilationTests.java +++ b/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 @@ class RecordCompilationTests extends CompilationTestCase { 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",