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 >
