Review request: update javac to properly output mandated parameters in MethodParameters attributes
Eric McCorkle
eric.mccorkle at oracle.com
Tue Feb 12 09:42:12 PST 2013
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/67362769/eric_mccorkle.vcf
More information about the compiler-dev
mailing list