Type annotations on inner type that is an array components
Werner Dietl
wdietl at gmail.com
Sun Jul 29 23:26:40 UTC 2018
Hi Bernard,
thanks for looking into this bug!
I created a bug:
https://bugs.openjdk.java.net/browse/JDK-8208470
Please let me know if I missed some information.
I was a bit worried about the different changes in your first patch.
Based on the missing method invocation you uncovered in your second
email, I made the patch below.
(It includes a few whitespace and style cleanups, and removes a dead method.)
Overall, `rewriteArrayType` is much easier to read after the change, I hope.
How do I run tier1 tests? I looked a bit in the compiler team
documentation, but didn't find current instructions.
Please let me know if I should send the patch in a different format or
if you have any comments!
Thanks,
cu, WMD.
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java
@@ -50,7 +50,6 @@
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type.ModuleType;
-import com.sun.tools.javac.code.TypeMetadata.Entry.Kind;
import com.sun.tools.javac.comp.Annotate;
import com.sun.tools.javac.comp.Attr;
import com.sun.tools.javac.comp.AttrContext;
@@ -423,8 +422,9 @@
return type;
}
- if (type.hasTag(TypeTag.ARRAY))
- return rewriteArrayType((ArrayType)type, annotations, pos);
+ if (type.hasTag(TypeTag.ARRAY)) {
+ return rewriteArrayType(typetree, (ArrayType)type,
annotations, pos);
+ }
if (type.hasTag(TypeTag.TYPEVAR)) {
return type.annotatedType(onlyTypeAnnotations);
@@ -536,15 +536,17 @@
*
* SIDE EFFECT: Update position for the annotations to be {@code pos}.
*/
- private Type rewriteArrayType(ArrayType type,
List<TypeCompound> annotations, TypeAnnotationPosition pos) {
+ private Type rewriteArrayType(JCTree typetree, ArrayType type,
+ List<TypeCompound> annotations, TypeAnnotationPosition pos) {
ArrayType tomodify = new ArrayType(type);
- ArrayType res = tomodify;
+ final ArrayType res = tomodify;
List<TypePathEntry> loc = List.nil();
// peel one and update loc
Type tmpType = type.elemtype;
loc = loc.prepend(TypePathEntry.ARRAY);
+ JCTree tmpTypeTree = ((JCArrayTypeTree) typetree).elemtype;
while (tmpType.hasTag(TypeTag.ARRAY)) {
ArrayType arr = (ArrayType)tmpType;
@@ -556,33 +558,17 @@
tmpType = arr.elemtype;
loc = loc.prepend(TypePathEntry.ARRAY);
+ tmpTypeTree = ((JCArrayTypeTree) tmpTypeTree).elemtype;
}
+ // Update positions
+ // All annotations share the same position; modify the first one.
+ TypeCompound tc = annotations.get(0);
+ tc.position = pos;
+ tc.position.location = loc;
+
// Fix innermost element type
- Type elemType;
- if (tmpType.getMetadata() != null) {
- List<TypeCompound> tcs;
- if (tmpType.getAnnotationMirrors().isEmpty()) {
- tcs = annotations;
- } else {
- // Special case, lets prepend
- tcs =
annotations.appendList(tmpType.getAnnotationMirrors());
- }
- elemType = tmpType.cloneWithMetadata(tmpType
- .getMetadata()
- .without(Kind.ANNOTATIONS)
- .combine(new TypeMetadata.Annotations(tcs)));
- } else {
- elemType = tmpType.cloneWithMetadata(new
TypeMetadata(new TypeMetadata.Annotations(annotations)));
- }
- tomodify.elemtype = elemType;
-
- // Update positions
- for (TypeCompound tc : annotations) {
- if (tc.position == null)
- tc.position = pos;
- tc.position.location = loc;
- }
+ tomodify.elemtype = typeWithAnnotations(tmpTypeTree,
tmpType, annotations, annotations, pos);
return res;
}
@@ -707,9 +693,7 @@
List<JCTree> path,
JCLambda currentLambda,
int outer_type_index,
- ListBuffer<TypePathEntry> location)
- {
-
+ ListBuffer<TypePathEntry> location) {
// Note that p.offset is set in
// com.sun.tools.javac.jvm.Gen.setTypeAnnotationPositions(int)
@@ -1399,37 +1383,25 @@
scan(tree.elems);
}
-
- private void findTypeCompoundPosition(JCTree tree, JCTree
frame, List<Attribute.TypeCompound> annotations) {
+ private void findPosition(JCTree tree, JCTree frame,
List<JCAnnotation> annotations) {
if (!annotations.isEmpty()) {
final TypeAnnotationPosition p =
- resolveFrame(tree, frame, frames,
currentLambda, 0, new ListBuffer<>());
- for (TypeCompound tc : annotations)
- tc.position = p;
- }
- }
-
- private void findPosition(JCTree tree, JCTree frame,
List<JCAnnotation> annotations) {
- if (!annotations.isEmpty())
- {
- final TypeAnnotationPosition p =
resolveFrame(tree, frame, frames, currentLambda,
0, new ListBuffer<>());
setTypeAnnotationPos(annotations, p);
}
}
- private void setTypeAnnotationPos(List<JCAnnotation>
annotations, TypeAnnotationPosition position)
- {
+ private void setTypeAnnotationPos(List<JCAnnotation>
annotations, TypeAnnotationPosition position) {
// attribute might be null during DeferredAttr;
// we will be back later.
for (JCAnnotation anno : annotations) {
- if (anno.attribute != null)
+ if (anno.attribute != null) {
((Attribute.TypeCompound)
anno.attribute).position = position;
+ }
}
}
-
@Override
public String toString() {
return super.toString() + ": sigOnly: " + sigOnly;
On Wed, Jul 25, 2018 at 2:04 PM B. Blaser <bsrbnd at gmail.com> wrote:
>
> Hi,
>
> On 19 July 2018 at 21:23, Alex Buckley <alex.buckley at oracle.com> wrote:
> > Hi Werner,
> >
> > On 7/18/2018 4:49 PM, Werner Dietl wrote:
> >>
> >> class ArrayOfInner {
> >> class Inner {}
> >>
> >> @TA(1) ArrayOfInner. @TA(2) Inner oi;
> >> @TA(3) ArrayOfInner. @TA(4) Inner [] oia;
> >> @TA(5) Inner i;
> >> @TA(6) Inner [] ia;
> >> }
> >>
> >>
> >> Compile with javac 8 and according to javap -v field ia has the
> >> annotation:
> >>
> >> ArrayOfInner$Inner[] ia;
> >> descriptor: [LArrayOfInner$Inner;
> >> flags: (0x0000)
> >> RuntimeVisibleTypeAnnotations:
> >> 0: #10(#11=I#21): FIELD, location=[ARRAY, INNER_TYPE]
> >>
> >> Compile with javac 9, 10, or a recent 11 build and ia has the annotation:
> >>
> >> ArrayOfInner$Inner[] ia;
> >> descriptor: [LArrayOfInner$Inner;
> >> flags: (0x0000)
> >> RuntimeVisibleTypeAnnotations:
> >> 0: #10(#11=I#21): FIELD, location=[ARRAY]
> >>
> >> Note the missing INNER_TYPE location.
> >>
> >> I would like to argue that the Java 8 behavior was correct.
> >
> >
> > Sounds right. Given source code that denotes the array type "Inner[]", the
> > question is whether the component type of that array type is a nested type
> > (ArrayOfInner.Inner) or not (just Inner). Logically, yes, it's a nested
> > type, so the target_path item deserves an INNER_TYPE entry.
> >
> > This feels like the kind of detail we've been round and round on in the
> > past. That said, I don't see any mails that would have led to javac changing
> > how it emits target_path between 8 and 9. The closest suspect is a
> > compiler-dev thread from Jan/Feb 2016 -- "Bug in encoding type annotations
> > for supertype_targets" -- which spoke about the target_path being omitted
> > entirely for an annotated nested type. (JDK-8148504 was filed against 8, but
> > it's still open, so it can't have caused any collateral damage for a field
> > type's annotation.)
> >
> > Alex
>
> Doing some archaeology in 'TypeAnnotations.java' I found a suspect
> change-set for JDK-8031744:
>
> https://bugs.openjdk.java.net/browse/JDK-8031744
>
> The array logic has been bundled in 'rewriteArrayType()' but the
> following line seems to have disappeared:
>
> http://hg.openjdk.java.net/jdk9/dev/langtools/rev/62e285806e83#l6.440
>
> This change-set might be the culprit but I haven't verified.
>
> Bernard
--
http://www.google.com/profiles/wdietl
More information about the compiler-dev
mailing list