[code-reflection] RFR: Simplify code generation for quotable lambdas not to require duplicate capture argument lists

[Maurizio Cimadamore]

On Fri, 4 Oct 2024 20:49:45 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> When translating quotable lambdas/method references, we generate two captured argument lists:
>> 
>> * the captured arguments to be used by the functional interface implementation;
>> * the captured arguments to be used by the quotable implementation
>> 
>> In most cases these two captured lists are identical, except for cosmetic differences.
>> This is even more true after the backend of javac underwent significant rearchitecture, to better support the Flexible Constructor Bodies feature.
>> 
>> This PR simplifies `ReflectMethods` so that a visitor now computes all the required captured variables upfront.
>> The captured variables are then inserted in the quoted block, so that the rest of `BodyScanner` will be able to resolve references to such variables w/o the need for ad-hoc logic.
>> 
>> One complication is that, under this new regime, we can no longer afford to treat captured variables whose value is constant (e.g. `final int k = 100`) as if they were true captures.
>> This is because neither `Lower` nor `LambdaToMethod` capture constant variables. Since we're reusing the same dynamic arguments as `LambdaToMethod` we need to make sure that `ReflectMethods` also does not capture constant arguments.
>> Special logic to avoid capture of constant arguments is baked into the new visitor.
>> 
>> I have also added a check in the visitor, so that if, inside a quotable lambda, we see a creation of a local class that is declared outside the lambda, an unsupported exception is thrown.
>> 
>> I will followup with a separate fix to deal with these cases.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ReflectMethods.java line 556:
> 
>> 554:             // add "this" capture (if needed)
>> 555:             if (lambdaCaptureScanner.capturesThis) {
>> 556:                 top.localToOp.put(currentClassSym, null); // @@@ just make translation happy
> 
> I think we can avoid this. For a quotable lambda we know the number of block parameters (captures) before we obtain the captured expressions from `top.localToOp`.
> 
> If `funcOp.parameters().size() == bodyScanner.stack.localToOp.size() + 1` then we know we need to first capture `this` expression. This is easier to do if we inline the call to `quotedCapturedArgs`.

Note that this logic is only really needed for the Quoted case, not the Quotable one. If we inline this directly into the Quoted path (as you suggested) perhaps this will be simplified away.

-------------

PR Review Comment: https://git.openjdk.org/babylon/pull/249#discussion_r1788325651