[code-reflection] RFR: BytecodeGenerator support of lambdas [v2]

[Paul Sandoz]

On Fri, 12 Apr 2024 07:20:13 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/reflect/code/bytecode/BytecodeGenerator.java line 741:
>> 
>>> 739:                         for (Value cv : op.capturedValues()) {
>>> 740:                             load(cv);
>>> 741:                         }
>> 
>> You should only need that when the resolved functional interface class is assignable to `Quotable`. This code appears to be quoting all lambdas?
>> 
>> In your test code you have a functional interface `Func` but it does not extend from `Quotable`, so when generating the bytecode you don't need to apply the quoting logic for lambda expressions targeted to that interface. The test could should be expanded ti also include say `interface FuncQuotable extends FuncQuotable { ... }`, and from such instances their code model can be obtained.
>
> Yes, now it is quoting all lambdas.
> We may restrict it to interfaces explicitly extending `Quotable`.
> Or we may use the ability of `LMF` to inject additional interfaces and make all lambdas generated from code models as `Quotable`.

We should only quote when the the FIs extend from `Quotable` or intersects with it. Otherwise, for non-quotable lambdas we are doing unnecessary work, deviating from what the compiler does, and potentially deviating from required behavior.

At the moment we don't support intersection types, but we plan to. I would suggest setting up the code path for quotable and non-quotable and switching based on whether the FI class is assignable. Later on we can enhance when intersection types are supported.

>> src/java.base/share/classes/java/lang/reflect/code/op/CoreOps.java line 1860:
>> 
>>> 1858:                     v -> switch (v) {
>>> 1859:                         case String s -> s;
>>> 1860:                         case null -> null;
>> 
>> What motivated this change?
>
> NPE when the attribute is missing. This passes the null name and works OK.

Ah, yes (also i think the change from `Map.of` to `new HashMap` is related to that). I think that's originating from building the code model to quote the lambda. Do you mind if i fix this separately? Lets add variable names to that code model (derived from the parameter index) to avoid this issue for now?

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

PR Review Comment: https://git.openjdk.org/babylon/pull/48#discussion_r1562637675
PR Review Comment: https://git.openjdk.org/babylon/pull/48#discussion_r1562626926