[code-reflection] RFR: Beef up reference resolution code and test

Paul Sandoz psandoz at openjdk.org
Thu Nov 13 17:52:14 UTC 2025


On Thu, 13 Nov 2025 17:13:31 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This PR improves the code for method/field/constructor resolution. To resolve fields and methods we have to perform two MH lookups, and find if any of them succeds. Since to code to do this is a bit convoluted, I have reimplemented the resolution support to use a more functional style (inspired by `Either` type).
> 
> I have also added several tests to make sure we test several lookup combinations, for instance/static method/fields, and also for super method resolution.
> 
> We will probably need more tests to cover the entire spectrum, but this should be at least a start.
> 
> I found 2 (!!) JDK issues when writing these tests, so some of the new tests contain workarounds.

Nice clean up. If you feel like it you could also convert `RecordTypeRefImpl` to a record as well, or i can do that in a separate PR.

The tests looks good enough for now, and should help is catch errors. We could more clearly separate out failure from success and the resolving type, but we could do that later if need be.

Just one small comment related to a redundant `instanceof` check.

src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/impl/ResolutionHelper.java line 31:

> 29:         HandleResolver<MethodHandle, Class<?>> FIND_GETTER = MethodHandles.Lookup::findGetter;
> 30:         HandleResolver<VarHandle, Class<?>> FIND_STATIC_VARHANDLE = MethodHandles.Lookup::findStaticVarHandle;
> 31:         HandleResolver<VarHandle, Class<?>> FIND_VARHANDLE = MethodHandles.Lookup::findVarHandle;

I think these would work just as well as static methods, without the memory/initialization footprint. Up to you.

src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/impl/ResolutionHelper.java line 72:

> 70:         if (t instanceof FunctionType ft) {
> 71:             return MethodRef.toNominalDescriptor(ft)
> 72:                     .resolveConstantDesc(l);

`t` is already an instance of `FunctionType`

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

Marked as reviewed by psandoz (Lead).

PR Review: https://git.openjdk.org/babylon/pull/682#pullrequestreview-3460903052
PR Review Comment: https://git.openjdk.org/babylon/pull/682#discussion_r2524392102
PR Review Comment: https://git.openjdk.org/babylon/pull/682#discussion_r2524337020


More information about the babylon-dev mailing list