Skip to content

/ loom

Permalink
Browse files

8236210: javac generates wrong annotation for fields generated from r… 

…ecord components

Reviewed-by: mcimadamore
  • Loading branch information
Vicente Romero
Vicente Romero committed Jan 24, 2020
1 parent 9e4830f commit 0f98701e8766b88a932e29ed2ba7dbd921e49786
26 src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java
View file
@@ -29,6 +29,7 @@
import java.lang.annotation.Inherited;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
@@ -58,6 +59,7 @@
import com.sun.tools.javac.jvm.*;
import com.sun.tools.javac.jvm.PoolConstant;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCAnnotation;
import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
import com.sun.tools.javac.tree.JCTree.Tag;
@@ -1274,6 +1276,9 @@ public RootPackageSymbol(Name name, Symbol owner, MissingInfoHandler missingInfo
/** the annotation metadata attached to this class */
private AnnotationTypeMetadata annotationTypeMetadata;

/* the list of any of record components, only non empty if the class is a record
* and it has at least one record component
*/
private List<RecordComponent> recordComponents = List.nil();

public ClassSymbol(long flags, Name name, Type type, Symbol owner) {
@@ -1471,15 +1476,24 @@ else if ((flags & RECORD) != 0)
return Flags.asModifierSet(flags & ~DEFAULT);
}

public RecordComponent getRecordComponent(VarSymbol field, boolean addIfMissing) {
public RecordComponent getRecordComponent(VarSymbol field) {
for (RecordComponent rc : recordComponents) {
if (rc.name == field.name) {
return rc;
}
}
return null;
}

public RecordComponent getRecordComponent(JCVariableDecl var, boolean addIfMissing) {
for (RecordComponent rc : recordComponents) {
if (rc.name == var.name) {
return rc;
}
}
RecordComponent rc = null;
if (addIfMissing) {
recordComponents = recordComponents.append(rc = new RecordComponent(PUBLIC, field.name, field.type, field.owner));
recordComponents = recordComponents.append(rc = new RecordComponent(var));
}
return rc;
}
@@ -1735,14 +1749,18 @@ public void setData(Object data) {
public static class RecordComponent extends VarSymbol implements RecordComponentElement {
public MethodSymbol accessor;
public JCTree.JCMethodDecl accessorMeth;
private final List<JCAnnotation> originalAnnos;

/**
* Construct a record component, given its flags, name, type and owner.
*/
public RecordComponent(long flags, Name name, Type type, Symbol owner) {
super(flags, name, type, owner);
public RecordComponent(JCVariableDecl fieldDecl) {
super(PUBLIC, fieldDecl.sym.name, fieldDecl.sym.type, fieldDecl.sym.owner);
this.originalAnnos = fieldDecl.mods.annotations;
}

public List<JCAnnotation> getOriginalAnnos() { return originalAnnos; }

@Override @DefinedBy(Api.LANGUAGE_MODEL)
@SuppressWarnings("preview")
public ElementKind getKind() {
20 src/jdk.compiler/share/classes/com/sun/tools/javac/code/SymbolMetadata.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2020, 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
@@ -267,25 +267,9 @@ private boolean isStarted() {
return lb.toList();
}

private List<Attribute.TypeCompound> removeFromTypeCompoundList(List<Attribute.TypeCompound> l, Attribute.TypeCompound compound) {
ListBuffer<Attribute.TypeCompound> lb = new ListBuffer<>();
for (Attribute.TypeCompound c : l) {
if (c != compound) {
lb.add(c);
}
}
return lb.toList();
}

public void remove(Attribute.Compound compound) {
public void removeDeclarationMetadata(Attribute.Compound compound) {
if (attributes.contains(compound)) {
attributes = removeFromCompoundList(attributes, compound);
} else if (type_attributes.contains(compound)) {
type_attributes = removeFromTypeCompoundList(type_attributes, (TypeCompound)compound);
} else if (init_type_attributes.contains(compound)) {
init_type_attributes = removeFromTypeCompoundList(init_type_attributes, (TypeCompound)compound);
} else if (clinit_type_attributes.contains(compound)) {
clinit_type_attributes = removeFromTypeCompoundList(clinit_type_attributes, (TypeCompound)compound);
} else {
// slow path, it could be that attributes list contains annotation containers, so we have to dig deeper
for (Attribute.Compound attrCompound : attributes) {
144 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
View file
@@ -2908,7 +2908,7 @@ private void validateAnnotation(JCAnnotation a, JCTree declarationTree, Symbol s
* the corresponding record component
*/
ClassSymbol recordClass = (ClassSymbol) s.owner;
RecordComponent rc = recordClass.getRecordComponent((VarSymbol)s, false);
RecordComponent rc = recordClass.getRecordComponent((VarSymbol)s);
SymbolMetadata metadata = rc.getMetadata();
if (metadata == null || metadata.isEmpty()) {
/* if not is empty then we have already been here, which is the case if multiple annotations are applied
@@ -2917,34 +2917,93 @@ private void validateAnnotation(JCAnnotation a, JCTree declarationTree, Symbol s
rc.appendAttributes(s.getRawAttributes().stream().filter(anno ->
Arrays.stream(getTargetNames(anno.type.tsym)).anyMatch(name -> name == names.RECORD_COMPONENT)
).collect(List.collector()));
rc.appendUniqueTypeAttributes(s.getRawTypeAttributes());
rc.setTypeAttributes(s.getRawTypeAttributes());
// to get all the type annotations applied to the type
rc.type = s.type;
}
}
}

if (a.type.tsym.isAnnotationType() && !annotationApplicable(a, s)) {
if (isRecordMember && (s.flags_field & Flags.GENERATED_MEMBER) != 0) {
/* so we have found an annotation that is not applicable to a record member that was generated by the
* compiler. This was intentionally done at TypeEnter, now is the moment strip away the annotations
* that are not applicable to the given record member
*/
JCModifiers modifiers = TreeInfo.getModifiers(declarationTree);
// lets first remove the annotation from the modifier
if (modifiers != null) {
ListBuffer<JCAnnotation> newAnnotations = new ListBuffer<>();
for (JCAnnotation anno : modifiers.annotations) {
if (anno != a) {
newAnnotations.add(anno);
/* the section below is tricky. Annotations applied to record components are propagated to the corresponding
* record member so if an annotation has target: FIELD, it is propagated to the corresponding FIELD, if it has
* target METHOD, it is propagated to the accessor and so on. But at the moment when method members are generated
* there is no enough information to propagate only the right annotations. So all the annotations are propagated
* to all the possible locations.
*
* At this point we need to remove all the annotations that are not in place before going on with the annotation
* party. On top of the above there is the issue that there is no AST representing record components, just symbols
* so the corresponding field has been holding all the annotations and it's metadata has been modified as if it
* was both a field and a record component.
*
* So there are two places where we need to trim annotations from: the metadata of the symbol and / or the modifiers
* in the AST. Whatever is in the metadata will be written to the class file, whatever is in the modifiers could
* be see by annotation processors.
*
* The metadata contains both type annotations and declaration annotations. At this point of the game we don't
* need to care about type annotations, they are all in the right place. But we could need to remove declaration
* annotations. So for declaration annotations if they are not applicable to the record member, excluding type
* annotations which are already correct, then we will remove it. For the AST modifiers if the annotation is not
* applicable either as type annotation and or declaration annotation, only in that case it will be removed.
*
* So it could be that annotation is removed as a declaration annotation but it is kept in the AST modifier for
* further inspection by annotation processors.
*
* For example:
*
* import java.lang.annotation.*;
*
* @Target({ElementType.TYPE_USE, ElementType.RECORD_COMPONENT})
* @Retention(RetentionPolicy.RUNTIME)
* @interface Anno { }
*
* record R(@Anno String s) {}
*
* at this point we will have for the case of the generated field:
* - @Anno in the modifier
* - @Anno as a type annotation
* - @Anno as a declaration annotation
*
* the last one should be removed because the annotation has not FIELD as target but it was applied as a
* declaration annotation because the field was being treated both as a field and as a record component
* as we have already copied the annotations to the record component, now the field doesn't need to hold
* annotations that are not intended for it anymore. Still @Anno has to be kept in the AST's modifiers as it
* is applicable as a type annotation to the type of the field.
*/

if (a.type.tsym.isAnnotationType()) {
Optional<Set<Name>> applicableTargetsOp = getApplicableTargets(a, s);
if (!applicableTargetsOp.isEmpty()) {
Set<Name> applicableTargets = applicableTargetsOp.get();
boolean notApplicableOrIsTypeUseOnly = applicableTargets.isEmpty() ||
applicableTargets.size() == 1 && applicableTargets.contains(names.TYPE_USE);
boolean isRecordMemberWithNonApplicableDeclAnno =
isRecordMember && (s.flags_field & Flags.GENERATED_MEMBER) != 0 && notApplicableOrIsTypeUseOnly;

if (applicableTargets.isEmpty() || isRecordMemberWithNonApplicableDeclAnno) {
if (isRecordMemberWithNonApplicableDeclAnno) {
/* so we have found an annotation that is not applicable to a record member that was generated by the
* compiler. This was intentionally done at TypeEnter, now is the moment strip away the annotations
* that are not applicable to the given record member
*/
JCModifiers modifiers = TreeInfo.getModifiers(declarationTree);
/* lets first remove the annotation from the modifier if it is not applicable, we have to check again as
* it could be a type annotation
*/
if (modifiers != null && applicableTargets.isEmpty()) {
ListBuffer<JCAnnotation> newAnnotations = new ListBuffer<>();
for (JCAnnotation anno : modifiers.annotations) {
if (anno != a) {
newAnnotations.add(anno);
}
}
modifiers.annotations = newAnnotations.toList();
}
// now lets remove it from the symbol
s.getMetadata().removeDeclarationMetadata(a.attribute);
} else {
log.error(a.pos(), Errors.AnnotationTypeNotApplicable);
}
modifiers.annotations = newAnnotations.toList();
}
// now lets remove it from the symbol
s.getMetadata().remove(a.attribute);
} else {
log.error(a.pos(), Errors.AnnotationTypeNotApplicable);
}
}

@@ -3221,10 +3280,20 @@ boolean isTypeAnnotation(Attribute a, boolean isTypeParameter) {
return targets;
}

@SuppressWarnings("preview")
boolean annotationApplicable(JCAnnotation a, Symbol s) {
Optional<Set<Name>> targets = getApplicableTargets(a, s);
/* the optional could be emtpy if the annotation is unknown in that case
* we return that it is applicable and if it is erroneous that should imply
* an error at the declaration site
*/
return targets.isEmpty() || targets.isPresent() && !targets.get().isEmpty();
}

@SuppressWarnings("preview")
Optional<Set<Name>> getApplicableTargets(JCAnnotation a, Symbol s) {
Attribute.Array arr = getAttributeTargetAttribute(a.annotationType.type.tsym);
Name[] targets;
Set<Name> applicableTargets = new HashSet<>();

if (arr == null) {
targets = defaultTargetMetaInfo();
@@ -3234,7 +3303,8 @@ boolean annotationApplicable(JCAnnotation a, Symbol s) {
for (int i=0; i<arr.values.length; ++i) {
Attribute app = arr.values[i];
if (!(app instanceof Attribute.Enum)) {
return true; // recovery
// recovery
return Optional.empty();
}
Attribute.Enum e = (Attribute.Enum) app;
targets[i] = e.value.name;
@@ -3243,55 +3313,55 @@ boolean annotationApplicable(JCAnnotation a, Symbol s) {
for (Name target : targets) {
if (target == names.TYPE) {
if (s.kind == TYP)
return true;
applicableTargets.add(names.TYPE);
} else if (target == names.FIELD) {
if (s.kind == VAR && s.owner.kind != MTH)
return true;
applicableTargets.add(names.FIELD);
} else if (target == names.RECORD_COMPONENT) {
if (s.getKind() == ElementKind.RECORD_COMPONENT) {
return true;
applicableTargets.add(names.RECORD_COMPONENT);
}
} else if (target == names.METHOD) {
if (s.kind == MTH && !s.isConstructor())
return true;
applicableTargets.add(names.METHOD);
} else if (target == names.PARAMETER) {
if (s.kind == VAR &&
(s.owner.kind == MTH && (s.flags() & PARAMETER) != 0)) {
return true;
applicableTargets.add(names.PARAMETER);
}
} else if (target == names.CONSTRUCTOR) {
if (s.kind == MTH && s.isConstructor())
return true;
applicableTargets.add(names.CONSTRUCTOR);
} else if (target == names.LOCAL_VARIABLE) {
if (s.kind == VAR && s.owner.kind == MTH &&
(s.flags() & PARAMETER) == 0) {
return true;
applicableTargets.add(names.LOCAL_VARIABLE);
}
} else if (target == names.ANNOTATION_TYPE) {
if (s.kind == TYP && (s.flags() & ANNOTATION) != 0) {
return true;
applicableTargets.add(names.ANNOTATION_TYPE);
}
} else if (target == names.PACKAGE) {
if (s.kind == PCK)
return true;
applicableTargets.add(names.PACKAGE);
} else if (target == names.TYPE_USE) {
if (s.kind == VAR && s.owner.kind == MTH && s.type.hasTag(NONE)) {
//cannot type annotate implicitly typed locals
return false;
continue;
} else if (s.kind == TYP || s.kind == VAR ||
(s.kind == MTH && !s.isConstructor() &&
!s.type.getReturnType().hasTag(VOID)) ||
(s.kind == MTH && s.isConstructor())) {
return true;
applicableTargets.add(names.TYPE_USE);
}
} else if (target == names.TYPE_PARAMETER) {
if (s.kind == TYP && s.type.hasTag(TYPEVAR))
return true;
applicableTargets.add(names.TYPE_PARAMETER);
} else
return true; // Unknown ElementType. This should be an error at declaration site,
// assume applicable.
return Optional.empty(); // Unknown ElementType. This should be an error at declaration site,
// assume applicable.
}
return false;
return Optional.of(applicableTargets);
}

Attribute.Array getAttributeTargetAttribute(TypeSymbol s) {

0 comments on commit 0f98701

Please sign in to comment.