Skip to content

Commit

Permalink
8288130: compiler error with AP and explicit record accessor
Browse files Browse the repository at this point in the history
Reviewed-by: jlahoda
  • Loading branch information
Vicente Romero committed Jun 24, 2022
1 parent 0828881 commit 53b37fe
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 184 deletions.
25 changes: 12 additions & 13 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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;
Expand All @@ -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; }
Expand Down
Expand Up @@ -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());

Expand Down
Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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));
Expand Down

3 comments on commit 53b37fe

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 53b37fe Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 53b37fe Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin the backport was successfully created on the branch GoeLin-backport-53b37fe1 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 53b37fe1 from the openjdk/jdk repository.

The commit being backported was authored by Vicente Romero on 24 Jun 2022 and was reviewed by Jan Lahoda.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-53b37fe1:GoeLin-backport-53b37fe1
$ git checkout GoeLin-backport-53b37fe1
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-53b37fe1

Please sign in to comment.