[code-reflection] RFR: ModuleOp generation from LambdaOp and FuncOp [v7]

[Paul Sandoz]

On Thu, 4 Dec 2025 21:04:53 GMT, Ruby Chen <duke at openjdk.org> wrote:

>> Add support for generating a ModuleOp from either a LambdaOp or a FuncOp. The given LambdaOp will be converted into the root FuncOp, and the given FuncOp will be assigned as the root FuncOp.
>> 
>> For `CoreOp.ModuleOp.ofLambdaOp()`, a unique name for the new root FuncOp must be passed as a parameter.
>
> Ruby Chen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed tests

src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/core/CoreOp.java line 370:

> 368:             stack.add(f);
> 369:             while (!stack.isEmpty()) {
> 370:                 CoreOp.FuncOp cur = stack.removeLast();

I find it easier to think in terms of pop/push. I see why you are doing this, to preserve the order of the call graph. Add a comment why you are working from the other end of the stack. You can accumulate into an array list (not a linked list) then do `stack.addAll(temp.reversed())` (also choose a better name than `temp`).

Alternatively use pop and push, accumulating into a list and then pushing on the stack like so:

            for (FuncOp f : invokedFuncs.reversed()) {
                work.push(f);
            }

src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/core/CoreOp.java line 372:

> 370:                 CoreOp.FuncOp cur = stack.removeLast();
> 371:                 if (!visited.add(cur)) continue;
> 372:                 funcs.add(convertInvokeToFuncCallOp(cur, l));

You are traversing the model and resolving methods twice, you can combine the two. If you want to keep it a separate method then pass the temporary list of the encountered invokes to reflectable methods.

src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/core/CoreOp.java line 390:

> 388:         public static CoreOp.ModuleOp ofLambdaOp(JavaOp.LambdaOp lambdaOp, MethodHandles.Lookup l, String lambdaName) {
> 389:             boolean methodNameExists = Arrays.stream(l.lookupClass().getMethods())
> 390:                     .anyMatch(method -> method.getName().equals(lambdaName));

I don't think that check is right. To do this properly we would have to check against the names of methods encountered in the call graph. In general we have another naming clash problem with overloaded methods. A simple approach is to always postfix the function name with the topologically sorted index e.g. "<name>_<index>"

test/jdk/java/lang/reflect/code/TestLambdaOps.java line 238:

> 236:         LambdaOp qop = (LambdaOp) Op.ofQuotable(lambda).get().op();
> 237:         FuncOp funcOp = qop.toFuncOp(null);
> 238:         System.out.println(funcOp.toText());

That's not making any assertions. Use the interpreter to invoke the function and lambda with the same arguments, and assert they produce the same result.

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

PR Review Comment: https://git.openjdk.org/babylon/pull/691#discussion_r2590919683
PR Review Comment: https://git.openjdk.org/babylon/pull/691#discussion_r2590906013
PR Review Comment: https://git.openjdk.org/babylon/pull/691#discussion_r2590930963
PR Review Comment: https://git.openjdk.org/babylon/pull/691#discussion_r2590935907