Type annotations on inner type that is an array components

[Werner Dietl]

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?

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

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

Thanks for adding the test cases!

Best,
cu, WMD.


On Mon, Jul 30, 2018 at 12:23 PM B. Blaser <bsrbnd at gmail.com> wrote:
>
> On 30 July 2018 at 12:50, B. Blaser <bsrbnd at gmail.com> wrote:
> > Hi Werner,
> >
> > On 30 July 2018 at 05:31, Werner Dietl <wdietl at gmail.com> wrote:
> >> Thanks to Liam for pointing out how to run the tests and, of course, I
> >> had missed a case: annotations on the array dimensions.
> >> I re-added the arrayTypeTree helper method that was removed in
> >>   http://hg.openjdk.java.net/jdk9/dev/langtools/rev/62e285806e83
> >> and tier1 tests are now clean, except for some failures that already
> >> existed for me.
> >> Updated patch below.
> >
> > I discovered the suspicious change-set after having submitted the
> > first patch as initial direction to search, but I was also septic that
> > it might be the final solution...
> > I'll take a look at your suggestion and give you my feedback soon.
> >
> > Maybe examining existing tests ([1],[2],...) and adding missing cases
> > would be interesting too?
>
> I've attached your latest patch updated as follows:
> * removed the unnecessary loop on array elements (replaced by
> recursion on 'typeWithAnnotations()')
> * added the 'onlyTypeAnnotations' parameter to 'rewriteArrayType()'
> * dropped cosmetic changes (indentation & dead code removal) which can
> be later reinstated
> * added some test cases
>
> Tier1 is OK (without any failure) but note the different output for
> 'T6747671.java' which probably means that the fix has side-effects.
>
> What do you think?
>
> Bernard
>
>
> > Thanks!
> > Bernard
> >
> > [1] http://hg.openjdk.java.net/jdk/jdk/file/b7a307084247/test/langtools/tools/javac/annotations/typeAnnotations/referenceinfos
> > [2] http://hg.openjdk.java.net/jdk/jdk/file/b7a307084247/test/langtools/tools/javac/annotations/typeAnnotations/referenceinfos/Fields.java
> >
> >> Best,
> >> cu, WMD.
> >>
> >>
> >>
> >> --
> >> http://www.google.com/profiles/wdietl



--
http://www.google.com/profiles/wdietl