Review request: update javac to properly output mandated parameters in MethodParameters attributes

[Eric McCorkle]

Thanks for the reviews.  I'll let you know if/when the test run makes it.

On 02/12/13 12:49, Maurizio Cimadamore wrote:
> Nice! It's good to go for me.
> 
> Maurizio
> 
> On 12/02/13 17:42, Eric McCorkle wrote:
>> On 02/12/13 11:56, Maurizio Cimadamore wrote:
>>> On 12/02/13 16:44, Eric McCorkle wrote:
>>>> Actually, it is necessary.  There are specific cases where a parameter
>>>> must be marked MANDATED, and it is marked SYNTHETIC in all others.
>>>>
>>>> Most notably, only the outer this in a *non-private* inner member class
>>>> constructor gets marked MANDATED.  If there's an outer this for a
>>>> static
>>>> or a private inner class constructor, it gets marked SYNTHETIC.
>>> Well, for a static inner class the method won't be called - but for a
>>> private inner it would (I must admit that I don't understand fully the
>>> rationale as to why private constructors are omitted from the mandated
>>> thing).
>> The choice seems somewhat arbitrary, but I have to do what the spec
>> says...
>>
>>> Anyway - it seems like you only need to check as to whether the owner of
>>> the method is private or not. Since this is a pretty short check, it can
>>> be inlined in the outerThisDef method.
>>>
>> I did something different.  I took the common codepaths and moved them
>> out to private methods, and made the old outerThisDef take a ClassSymbol
>> argument.
>>
>> Rationale: The MethodSymbol variant needs a MethodSymbol, not a Symbol
>> to do what it needs to do.  It needs it in two separate places: to
>> figure out the flags, and to add to extraParameters.  Doing this inline
>> in the same function leads to two separate if-instanceof-casts, and
>> generally uglies things up.
>>
>> The newest webrev has the changes.
>>
>> However, given the amount of messing with these codepaths that's gone
>> on, I've submitted another JPRT run.
>>
>>> Maurizio
>>>> On 02/12/13 11:34, Maurizio Cimadamore wrote:
>>>>> On 12/02/13 16:22, Maurizio Cimadamore wrote:
>>>>>> On 12/02/13 16:17, Eric McCorkle wrote:
>>>>>>> Maybe make the one for ClassSymbol take a ClassSymbol as an
>>>>>>> argument as
>>>>>>> opposed to a Symbol?
>>>>>> Why do we need to replicate all that code?
>>>>> Also, I'm pretty sure that you don't need outerThisIsMandated anymore,
>>>>> since outerThisDef is explicitly called to add an extra 'this'
>>>>> argument
>>>>> to the constructor of an inner class.
>>>>>
>>>>> Maurizio
>>>>>> Maurizio
>>>>>>> On 02/12/13 10:52, Maurizio Cimadamore wrote:
>>>>>>>> On 12/02/13 14:53, Eric McCorkle wrote:
>>>>>>>>> No, actually, that variant has to be there in order to add the
>>>>>>>>> outer
>>>>>>>>> this field in a ClassSymbol.
>>>>>>>> The overloading on Symbol/MethodSymbol is  very hard to parse -
>>>>>>>> i.e.
>>>>>>>> it's hard to tell whether a client would call one or the other.
>>>>>>>> What
>>>>>>>> I'm
>>>>>>>> suggesting is that, since both methods seem to share 99% of the
>>>>>>>> code,
>>>>>>>> you add the extra logic to the old method guarded by a check
>>>>>>>> (which I
>>>>>>>> think is sym.kind == MTH - i.e. if the symbl is a method).
>>>>>>>>
>>>>>>>> Maurizio
> 
-------------- 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/20130212/f3f63bb9/eric_mccorkle.vcf