Approved thanks Eric,
cheers /Joel On 2014-11-10, Eric McCorkle wrote: > Ok, the test has been revised as you suggested. > > A new version is posted (webrev.03). > > On 11/10/14 06:45, Joel Borggrén-Franck wrote: > > Hi Eric, > > > > The changes to Executable.java and Parameter.java looks good. The test > > could still need some cleaning up: > > > > @summary should be before "action tags", put it right after @bug > > > > I don't think you need a separate file for > > InnerClassToStringNoParameters.java , should be able to accomplish the same > > with jtreg tags, > > > > * @clean InnerClassToString > > * @compile -parameters InnerClassToString.java > > * @run main InnerClassToString > > * @clean InnerClassToString > > * @compile InnerClassToString.java > > * @run main InnerClassToString > > > > I think this is cleaner, YMMV. > > > > The main method suffers from a lot of code duplication (and an absence > > of line breaks), you can extract a test method since the test just > > operates on two arrays of Type[] and call it with the parameter type > > array and the answer array. > > > > cheers > > /Joel > > > > On 2014-11-05, Eric McCorkle wrote: > >> Thanks, Joel. I've applied your fixes and re-upped the webrev. > >> > >> On 11/05/14 10:10, Joel Borggrén-Franck wrote: > >>> Hi Eric, > >>> > >>> I think caching the real parameter type array on Executable is the wrong > >>> thing to do considering we can have a lot of instances of Executable but > >>> fairly few will be examined for parameters and the vast majority of those > >>> will never have this issue. > >>> > >>> Also, the tests are a bit minimal, I would expect you to check that the > >>> type of the parameter is correct. The tests should also have a @bug line. > >>> > >>> I think you are overly cautious here. I’m not sure we need to work around > >>> potential bugs in non-Java compilers. If language Y inserts > >>> synthetic/mandated parameters anywhere else than as a prefix they better > >>> generate a correct Signature (or non at all). This is a discussion we can > >>> defer however. > >>> > >>> If you flesh out the test and remove the caching I think this is a fine > >>> fix for both 8u and 9. > >>> > >>> cheers > >>> /Joel > >>> > >>> On 31 okt 2014, at 17:15, Eric McCorkle <eric.mccor...@oracle.com> wrote: > >>> > >>>> Hello, > >>>> > >>>> Please review this patch which fixes issues that arise with > >>>> getGenericParameterTypes() and getAnnotatedParameterTypes() when there > >>>> are generic signatures and synthetic parameters. > >>>> > >>>> Please note that a complete fix is not possible for all cases. See > >>>> discussion on https://bugs.openjdk.java.net/browse/JDK-8062582 for > >>>> details. > >>>> > >>>> This patch will cause Executable.getAnnotatedParameterTypes(), > >>>> Parameter.getAnnotatedType(), and Parameter.getParameterizedType() to > >>>> report the correct types in the following cases: > >>>> > >>>> * No generic signature is present. > >>>> * Both a generic signature and method parameters information are present > >>>> * A generic signature is present, but method parameters information is > >>>> not present, but the number of parameters in the generic signature and > >>>> the number of parameters in the method descriptor are the same. > >>>> > >>>> In the problematic case, where there is a generic signature, no method > >>>> parameters information, and the generic signature does not match the > >>>> method descriptor, these methods will return the correct /non/-generic > >>>> type, as there is no general way of associating parameters in the > >>>> generic signature with those in the method descriptor in this case. > >>>> > >>>> Please also note that there is currently a bug in javac which causes > >>>> type annotations' parameter indexes to be wrong when synthetic > >>>> parameters are generated: > >>>> https://bugs.openjdk.java.net/browse/JDK-8029012. > >>>> > >>>> The bug report is here: > >>>> https://bugs.openjdk.java.net/browse/JDK-8055063 > >>>> > >>>> The webrev is here: > >>>> http://cr.openjdk.java.net/~emc/8055063/ > >>> > > > >> begin:vcard > >> fn:Eric McCorkle > >> n:McCorkle;Eric > >> org:Oracle Corporation;Java Platform Group > >> adr:BUR02-2117;;35 Network Drive;Burlington;MA;01803;United States > >> email;internet:eric.mccor...@oracle.com > >> title:Senior Member of Technical Staff > >> tel;work:1-781-442-1028 > >> tel;cell:404-644-6360 > >> version:2.1 > >> end:vcard > >> > > > begin:vcard > fn:Eric McCorkle > n:McCorkle;Eric > org:Oracle Corporation;Java Platform Group > adr:BUR02-2117;;35 Network Drive;Burlington;MA;01803;United States > email;internet:eric.mccor...@oracle.com > title:Senior Member of Technical Staff > tel;work:1-781-442-1028 > tel;cell:404-644-6360 > version:2.1 > end:vcard >
