Review request for JDK-8020745: Suspicious MethodParameters attribute generated for local classes capturing local variables

Eric McCorkle eric.mccorkle at oracle.com
Wed Aug 21 14:00:59 PDT 2013


I have addressed these issues and uploaded a new webrev.  Crucible
reviews are also complete.

On 08/20/13 21:17, Alex Buckley wrote:
> - Method makeAnonExtendsInner does not return an instance of an
> anonymous class whose superclass is inner. InnerList is a local class,
> and its superclass ArrayList is top level. Also, you should document the
> kinds of classes for which you're exercising parameter generation.
> 
> - Encapsulate! The "expected" variables like Inner_names should be
> closer to the classes declaring the "actual" variables (i.e. constructor
> parameters) of interest. checkConstructor should merely have to ask that
> class to check itself.
> 
> Alex
> 
> On 8/20/2013 3:36 PM, Eric McCorkle wrote:
>> A new webrev has been uploaded which addresses these issues.
>> http://cr.openjdk.java.net/~emc/8020745/
>>
>> On 08/19/13 20:28, Alex Buckley wrote:
>>> The AnonymousParameters.java test is very confusing. Two classes with
>>> main methods in the same compilation unit?!
>>>
>>> Bug 8020745 indicates a mismatch between parameter names and types, but
>>> this test only checks names. Rather than relying on the order of names
>>> from the JVM to match the 'names' array, you should match up names and
>>> types explicitly.
>>>
>>> List is a bad name for a helper class, and the @summary is not very
>>> helpful.
>>>
>>> Most importantly, there are no anonymous classes here - List is a local
>>> class. The test should have an anonymous class too - in fact, an
>>> anonymous class whose superclass is not inner and an anonymous class
>>> whose superclass is inner. (See the notes in 8misc.pdf 2.2 8.8.9 for why
>>> that matters.)
>>>
>>> Alex
>>>
>>> On 8/19/2013 4:31 PM, Eric McCorkle wrote:
>>>> Hello,
>>>>
>>>> Please review this patch, which causes captured locals to be added to
>>>> the end of a MethodParameters attribute, like they should.
>>>>
>>>> The webrev is here:
>>>> http://cr.openjdk.java.net/~emc/8020745/
>>>>
>>>> The bug report is here:
>>>> http://bugs.sun.com/view_bug.do?bug_id=8020745
>>>>
>>>> This review is also on the new Crucible system.
>>>>
>>>> Thanks,
>>>> Eric
>>>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: eric_mccorkle.vcf
Type: text/x-vcard
Size: 314 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20130821/105d71a5/eric_mccorkle.vcf 


More information about the compiler-dev mailing list