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 >> >
