I just deleted and recreated the file locally to avoid this now. No idea why the webrev command picked it up as a file change.
I fixed the patch here: http://cr.openjdk.java.net/~winterhalter/8202473/webrev.02/ As for the possible refactoring: I had investigated it but in the factories for wildcard and parameterized types, the parameters are still provided with different arguments. Am Sa., 6. Juni 2020 um 11:44 Uhr schrieb Joel Borggrén-Franck < joel.borggren.fra...@gmail.com>: > Hi Rafael, > > Let's try to get this into 15, > > > http://cr.openjdk.java.net/~winterhalter/8202473/webrev.00/test/jdk/java/lang/annotation/typeAnnotations/TypeVariableBoundParameterIndex.java.udiff.html > looks strange did you unintentionally remove the test for 8202469? The > issues are orthogonal so I'd like to keep the cases you developed for the > previous fix. > > I think the fix can lead to further refactorings since this was the last > use of buildAnnotatedType where parameter 4 and 5 wasn't identical. This > can be followed up later. > > cheers > /Joel > > On Tue, Apr 7, 2020 at 8:52 PM Rafael Winterhalter <rafael....@gmail.com> > wrote: > >> I created this webrev to also fix the second part that was previously >> fixed as part of 8202469: >> http://cr.openjdk.java.net/~winterhalter/8202473/webrev.00/ - this would >> be the second part of the adjustment. >> >> This is reported in https://bugs.openjdk.java.net/browse/JDK-8202473 and >> was closed as a duplicate. With this cleaner solution, the duplication does >> however no longer apply. >> >> Am Mo., 6. Apr. 2020 um 14:01 Uhr schrieb Joel Borggrén-Franck < >> joel.borggren.fra...@gmail.com>: >> >>> Many thanks to Vicente for sponsoring this. I'll start to look at the >>> second part shortly. >>> >>> cheers >>> /Joel >>> >>> On Mon, Mar 16, 2020 at 9:44 PM Joel Borggrén-Franck < >>> joel.borggren.fra...@gmail.com> wrote: >>> >>>> Looks good to me! >>>> >>>> I'll see if I can find a sponsor for this. >>>> >>>> cheers >>>> /Joel >>>> >>>> On Sat, Mar 7, 2020 at 2:15 AM Rafael Winterhalter < >>>> rafael....@gmail.com> wrote: >>>> >>>>> 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: >>>>> http://cr.openjdk.java.net/~winterhalter/8202469/webrev.01/ >>>>> >>>>> Thanks, Rafael >>>>> >>>>> Am So., 16. Feb. 2020 um 10:51 Uhr schrieb Joel Borggrén-Franck < >>>>> joel.borggren.fra...@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....@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....@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.fu...@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 winterhal...@cr.openjdk.java.net: ) >>>>>>>>> >>>>>>>>> best regards and welcome! >>>>>>>>> >>>>>>>>> -- daniel >>>>>>>>> >>>>>>>>
