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

[Maurizio Cimadamore]

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