8202469 / 8202473: Correct type annotation resolution for class type variables

Rafael Winterhalter rafael.wth at gmail.com
Sat Mar 7 01:15:11 UTC 2020

I finally found the time to look at this again, sorry for the delay.

Thank you for your comments. I had the chance to better look into the
problem and simplify the code a bit more and also reduced the scope of the
fix to a single problem. I also added test cases that should be exhaustive
for all possible scenarios and adjusted the code comment.

I uploaded the adjusted patch as a webrev:

Thanks, Rafael

Am So., 16. Feb. 2020 um 10:51 Uhr schrieb Joel Borggrén-Franck <
joel.borggren.franck at gmail.com>:

> Hi Rafael,
> Thanks for reaching out and reminding me of this!
> I managed to find some time to look into 8202469 during the weekend. By
> swapping place of  the TVars in the test I managed to isolate this without
> depending on 8202473, take a look at my hacky copy-paste update to your
> test at http://cr.openjdk.java.net/~jfranck/8202469/ .
> The comment on lines 269-276 needs to be updated. Perhaps include a table
> with the start index depending of the type? Also referencing JVMLS 4.4 for
> the structure of the bound might be instructive for future readers.
> My current understanding is that there are two problems with the code on
> (the original) lines 277-287:
> 1) Type Variables should have index 0 while getting index 1 due to lines
> 279-280.
> 2) Bounds that are instances of ParameterizedType always gets index 1 but
> if the first bound represents a Class type the index should be 0.
> Does this make sense?
> Can you make the if-switches positive? I find my original code with the
> negative tests hard to read and the update doesn't help.
> Also can you expand the test to cover the different kinds of bounds
> mentioned in 4.4 and also test Type Variables on methods, I suspect javac
> treats method (and constructor) tvars similarly but there have been bugs ...
> TL;DR please update the webrev with:
> - Split out test and fix for 8202469
> - More test coverage of different kinds of bounds
> - Test for method tvars
> - See if you can rework the logic to have (mostly) positive tests in if
> switch
> Thanks again for looking into this, I'll start looking into 8202473, I
> think the fix for that one opens up a bigger rework of the code which is
> why I want to separate the two.
> cheers
> /Joel
> On Sun, Aug 4, 2019 at 10:12 PM Rafael Winterhalter <rafael.wth at gmail.com>
> wrote:
>> Hi,
>> appologies for the delay, I only managed to get my patched up to webrev
>> today (http://cr.openjdk.java.net/~winterhalter/). It's three patches in
>> total now, for the third one I could not add a test case, it would require
>> to compile an annotation property against an enumeration type and load it
>> against another class where the enumeration was turned into an annotation.
>> I struggled a bit with jtreg to make it work but I cannot see myself
>> complete this in a meaningful amount of time. However, I hope that the
>> patch and the error it solves are straightforward to see.
>> I only submitted a patch for 8202469. 8202473 is solved by it. It's two
>> bugs but they are a symptom of the same oversight.
>> I hope this helps you to review it but let me know if you have any
>> questions or if I should further adjust my patch.
>> Best regards, Rafael
>> Am Fr., 2. Aug. 2019 um 12:18 Uhr schrieb Rafael Winterhalter <
>> rafael.wth at gmail.com>:
>>> Thanks Daniel,
>>> I did some work on Glassfish a bunch of years ago, I had no idea.
>>> I will try to split up the changes (I fixed another bug in annotation
>>> processing yesterday) and upload everything there.
>>> Best regards, Rafael
>>> Am Fr., 2. Aug. 2019 um 11:59 Uhr schrieb Daniel Fuchs <
>>> daniel.fuchs at oracle.com>:
>>>> Hi Rafael,
>>>> On 02/08/2019 09:00, Joel Borggrén-Franck wrote:
>>>> > I can host webrevs for you on cr to make it easier for other
>>>> reviewers, if
>>>> > you also send me the patches or webrefs off-list to get around the
>>>> > attachment stripping I can upload them to cr.
>>>> I see that you are a JDK project author, so you already own a page
>>>> on cr (http://cr.openjdk.java.net/~winterhalter/) where you can
>>>> upload webrevs (e.g. using `scp` as in:
>>>> $ scp -r webrev-NNNNN.01 winterhalter at cr.openjdk.java.net: )
>>>> best regards and welcome!
>>>> -- daniel

More information about the core-libs-dev mailing list