Type annotations on inner type that is an array components

B. Blaser bsrbnd at gmail.com
Mon Jul 30 21:59:33 UTC 2018


On 30 July 2018 at 20:19, Werner Dietl <wdietl at gmail.com> wrote:
> In this part:
>
>              // Update first position
>              for (TypeCompound tc : annotations) {
>                  if (tc.position == null)
>                      tc.position = pos;
>                 tc.position.location =
> tc.position.location.prepend(TypePathEntry.ARRAY);
>
> I think you can simplify to:
>
>     TypeCompound tc = annotations.get(0);
>     tc.position = pos;
>     tc.position.location = tc.position.location.prepend(TypePathEntry.ARRAY);
>
> You know that annotations is non-empty, so there is always at least
> the first element that you need to update.
> I'm not sure why the null check was there. Even if the position is
> already non-null, don't we want to set it to the position we
> determined?

I wasn't sure of this, that's why I conservatively kept this check.
But I agree that 'tc = annotations.get(0)' would be possible too.

> Should we change `arrayTypeTree` to `arrayTypeTreeElement` or some
> such and put the `.elemtype` access in the helper method?

Why not but does this really improve readability?

> The change to T6747671.out seems ok to me - the error message now
> contains the annotation on the type use.

I've attached a variant of the fix without any side-effect (the new
elements discussed above are not yet included but I reinstated the
dead code removal).
Any comment about this variant (and 'T6747671' expected output) is welcome.

> Thanks for adding the test cases!

Thank you too,
Bernard

> Best,
> cu, WMD.
>
>
>
>
>
> --
> http://www.google.com/profiles/wdietl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jdk8208470c.patch
Type: text/x-patch
Size: 8984 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180730/d3bd8b3c/jdk8208470c.patch>


More information about the compiler-dev mailing list