RFR JDK-8208470: Type annotations on inner type that is an array component

[Liam Miller-Cushon]

Hi Bernard,

On Tue, Sep 18, 2018 at 6:42 AM B. Blaser <bsrbnd at gmail.com> wrote:

> Please review the following fix for [1]:
> http://cr.openjdk.java.net/~bsrbnd/jdk8208470/webrev.00/


I'm not a Reviewer, but this approach looks good to me overall.

A couple of minor notes:

There was some previous discussion (
http://mail.openjdk.java.net/pipermail/compiler-dev/2018-July/012264.html)
about this part:

```
// Update positions
for (TypeCompound tc : annotations) {
    if (tc.position == null)
        tc.position = pos;
}
```

As far as I can tell it's redundant. Adding the following assertion to
`typeWithAnnotations` doesn't break any tests:

```
for (TypeCompound tc : annotations) {
    Assert.check(tc.position == pos);
}
```

... which matches the comment in the logic for nested types in
`typeWithAnnotations` that "all annotations share the same position; modify
the first one", and suggests that could be cleaned up too. (
http://hg.openjdk.java.net/jdk/jdk/file/2626982cf4f7/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeAnnotations.java#l525
)

```
-                    // All annotations share the same position; modify the
first one.
-                    Attribute.TypeCompound a = annotations.get(0);
-                    TypeAnnotationPosition p = a.position;
-                    p.location = p.location.appendList(depth.toList());
+                    pos.location = pos.location.appendList(depth.toList());
```

I'm wondering if maybe we can remove that part of `rewriteArrayType`?

nit: arrayTypeTree is inconsistent about asserting on unexpected types vs.
just performing casts (e.g. it would throw a CCE for a non-array
JCAnnotatedType tree). Unless that assertion is defending against something
specific, consider just:

```
private JCArrayTypeTree arrayTypeTree(JCTree typetree) {
  if (typetree.getKind() == JCTree.Kind.ANNOTATED_TYPE) {
    typetree = ((JCAnnotatedType) typetree).underlyingType;
  }
  return (JCArrayTypeTree) typetree;
}
```

On Wed, Dec 12, 2018 at 6:56 AM B. Blaser <bsrbnd at gmail.com> wrote:

> Now, the question of Werner to be addressed in a separate issue is why
> isn't there any @TA on "new" expression warnings?
>

I did some debugging, and it looks like the rawtypes diagnostic
pretty-prints the attributed type, and at that point the type annotations
in `B [] arr = new @TA B [0];` have been deferred and not yet added to the
type (see Annotate#flush).

I'm not sure what the principled fix is, but forcing the deferred
annotations to be processed before emitting the diagnostics (by adding in
an explicit call to `flush()`) makes the diagnostics consistently include
type annotation information. In my opinion the effect on diagnostic quality
is fairly minor, and fixing that inconsistency could safely be deferred.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20181212/fce9b6f3/attachment.html>