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

Alex Buckley alex.buckley at oracle.com
Tue Aug 20 18:17:38 PDT 2013


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


More information about the compiler-dev mailing list