[jvm] auto-detect Java SAM interfaces; honor SAM in overload resolution #12901
Conversation
|
@EliteMasterEric would this help you with your jvm project(s)? Can you check if your project run into issues with this (either at compile time or runtime) with corresponding nightlies? |
819c31b to
8e768fc
Compare
Two related changes that together make Haxe match javac's lambda-vs-SAM conversion rules on the JVM target. 1. Structural SAM detection in javaModern.ml Previously, only interfaces carrying @FunctionalInterface were tagged CFunctionalInterface and so eligible for implicit conversion from a function value. The annotation is documentation in javac too — what makes a type lambda-convertible is having exactly one abstract instance method, per JLS §9.8. The Android SDK rarely annotates its listeners, which forced every consumer of the JVM target to hand-roll adapter classes for View.OnClickListener, DialogInterface.OnClickListener, MediaPlayer.OnCompletionListener and so on. This patch auto-tags any .jar-imported interface as @:functionalInterface when it has exactly one abstract instance method, excluding: - static / default / private / synthetic methods, - constructors, - Object members re-declared as abstract (equals/hashCode/toString, matched by name+arity since their signatures on j.l.Object are unambiguous — JLS §9.8 excludes these from the SAM count). The explicit @FunctionalInterface annotation path is preserved. 2. SAM-aware overload candidate filtering overloadResolution.ml's unify_cf used strict Type.unify to filter candidates before the cast layer could fire. A function value passed to an overload whose parameter is a SAM class would be discarded at the filter step. Now, when strict unify fails, the filter retries via the SAM conversion path — unifying the function's TFun against the SAM method's signature, mirroring what AbstractCast does at cast time. Only candidates whose SAM signature actually matches the function shape survive, so this isn't a blind accept. Two supporting fixes round this out: - overloads.ml rate_conv: added a TInst(SAM), TFun case so specificity ranking works when two SAM-parametered overloads both accept the same function. The rate descends through the SAM method's signature rather than raising Not_found, which previously discarded the candidate during reduce_compatible. - tOther.ml get_singular_interface_field and the parallel walker in genjvm.ml's check_functional_interface now exclude equals/hashCode/ toString by name+arity, matching the new exclusion in javaModern so interfaces like Comparator (abstract equals re-declared) — and the Android SDK's various 'override equals' interfaces — are still detected as SAMs at both typer and codegen stages. Tests: - tests/unit/.../IssueFunctionalInterfaceOverload.hx exercises the overload disambiguation path with two @:functionalInterface interfaces of disjoint shape (Int,Int -> Int vs String -> String). - tests/misc/jvm/projects/StructuralSam/ ships a .java file with six interface shapes — plain SAM, abstract-equals re-declaration, default/static method skipping, multi-abstract non-SAM, generic-arg SAM, and overloaded call sites — none annotated @FunctionalInterface. Runs the generated jar end-to-end to verify both type-checking and runtime closure-adapter dispatch. Limitations matching javac: - Abstract-class SAMs (e.g. android.content.BroadcastReceiver) remain unsupported; javac doesn't allow lambda conversion to those either. - Two SAM-parametered overloads where both signatures would accept the same function value remain ambiguous and require explicit disambiguation.
Auto-tagging every structurally-SAM interface as @:functionalInterface swept in sun.reflect.ConstructorAccessor, sun.nio.ch.Interruptible, sun.reflect.generics.tree.TypeTree and similar JDK-internal classes. The existing JFI matcher then made every signature-matching closure declare `implements sun.reflect.ConstructorAccessor`, which compiles fine against the extern jar but fails to defineClass at runtime on JDK 9+ — sun.* was moved to jdk.internal.* and is inaccessible to application code. This broke unit tests at startup: Issue5556 was the first class to trigger loading the affected closure adapter, raising NoClassDefFoundError before any test ran. User code can't legitimately target these internal packages anyway, so the fix is to exclude sun.*, com.sun.*, and jdk.internal.* from structural detection. Classes that actually carry @FunctionalInterface are still honored (none of the affected internal classes do, so this is safe). Issue5556 serves as the regression test — its closure adapter previously implemented sun.reflect.ConstructorAccessor and now no longer does.
Structural SAM detection tags every single-abstract-method interface on
the --java-lib classpath as @:functionalInterface, and a closure then
implements every one of them whose signature structurally matches. That
includes interfaces transitively loaded from the SDK jar but never named
in user code — e.g. compiling against android-34 drags in
android.os.OutcomeReceiver (API 33) and SplashScreen.OnExitAnimationListener
(API 31). A closure implementing those hard-fails class linking
(NoClassDefFoundError) the moment it loads on an older runtime, crashing
the app at activity instantiation.
Restrict the promiscuous binding to interfaces the program genuinely
converts a function to:
- new com.functional_interfaces_used set (shared across cloned contexts,
exposed on Gctx.t for the generator);
- AbstractCast records each interface as its SAM-conversion branch fires,
keyed by the @:native path since Native.apply_native_paths rewrites
cl_path between typing and generation;
- genjvm's Preprocessor only registers an interface in
gctx.functional_interfaces when it is in that set, resolved by physical
class identity in the modules loop (before check_path can rewrite the
path again).
StructuralSam gains an Unused interface — structurally identical to
OnClick but never used as a conversion target — and asserts a closure
implements OnClick but not Unused.
The functional_interfaces_used set populated by AbstractCast only covers interfaces reached through an implicit SAM conversion typed from source. That misses two cases: an explicit `cast` of a closure to a functional interface bypasses AbstractCast entirely, and an `--hxb-lib` build never re-runs typing so the set is empty — closures then fail to implement the interface and hard-cast at runtime (Issue9576, Issue11236, IssueFunctionalInterfaceOverload under the hxb-lib CI configs). Additionally scan the AST of non-extern types in genjvm's Preprocessor: field/static/constructor signatures and their cf_overloads, plus the etype of every sub-expression (which catches the TFun type of a called extern method whose parameter is a SAM interface). This is independent of how the module was loaded. Restricting the scan to non-extern types keeps incidental SAM interfaces from a --java-lib classpath out of the set unless user code actually references them.
|
I understand what we're trying to do here, but it seems a little messy because there's no clear authority on what is and isn't considered a SAM interface. The java loader initially determines it, then the typer basically verifies it via I'm aware that this PR doesn't really introduce this mess and merely amplifies it, but I would still like to look into a cleaner approach here. Having |
…_field The JLS §9.8 SAM rule (exclude default methods and Object members) was duplicated between tOther and genjvm; the latter now just calls the shared helper.
AbstractCast was writing into a Hashtbl on the common context to track which SAM interfaces a program converts a function to, with @:native re-keying to bridge the typing↔generation path rewrite. The genjvm preprocessor already walks the AST and collects every SAM-typed TInst it sees, which strictly subsumes the AbstractCast set (it also catches explicit casts and --hxb-lib builds). Remove the redundant bookkeeping and the field on common.context / Gctx.t.
Claude's answer before implementingLooking at the diff, the mess concentrates in two axes — what counts as a SAM and which SAMs the program uses. Both can be tightened significantly. What's actually duplicatedSAM-ness rule appears three times, each with its own copy of the JLS §9.8 Object-member exclusion:
Usage tracking appears twice:
Proposed cleanup1. One module owns "is this a SAM, and what's its method?"Add Then:
2. Drop
|
|
Much better! One thing I'm slightly worried about is |
Avoid an O(n*m) lookup over the SAM-used list during the interface preprocessing loop. The list is finalized into a Hashtbl after loop 1 completes (and therefore after check_path's cl_path rewrites), so keying by cl_path is stable.
|
Claude seems to think the whole CI seems ok with removing it, at least (commit / CI run). Should I cherry-pick that commit here? |
I don't understand that part because |
|
It got confused about #11549 somehow, but still insists the lut wasn't doing much:
|
The LUT memoized get_singular_interface_field per class, populated by typeloadFields.check_functional_interface and re-checked lazily by AbstractCast (#11549, for display-mode flows where init_class hadn't run yet by the time a SAM conversion was queried). It was a global mutable hashtbl on common.context whose only reader was AbstractCast. The cached computation is a walk over a SAM interface's field list — typically under 10 entries, and short-circuits on the second non-default method, so the perf delta from recomputing is well below noise. Doing that directly in AbstractCast collapses the two load paths into one and removes the field from common.context. Also drops the now-no-op typeloadFields.check_functional_interface (its only effect was populating the LUT) and the bridge call that re-invoked it from init_class when the flag was set via @:functionalInterface meta in typeloadModule.
|
I'm getting it to benchmark the change with pessimistic scenarios, and it quickly hit the JVM method-size limit instead 😅 |
|
Yes I think this is fine, the argument just didn't seem accurate. One last cosmetic change I'd like to make is to have that |
Move the SAM-usage AST scan out of preprocess and into its own top-level function in the Preprocessor module that returns the path-keyed hashtbl. The scan now runs after the check_path loop so cl_paths are stable when we record them.
|
Something like that? |
|
The standalone diff looks a bit strange but I think that's right. |
|
Looks better in the PR diff indeed :) |
Two related changes that together make Haxe match javac's lambda-vs-SAM
(Single Abstract Method) conversion rules on the JVM target.
Previously, only interfaces carrying
@FunctionalInterfacewere taggedCFunctionalInterface and so eligible for implicit conversion from a
function value. The annotation is documentation in javac too — what
makes a type lambda-convertible is having exactly one abstract instance
method, per JLS §9.8. The Android SDK rarely annotates its listeners,
which forced every consumer of the JVM target to hand-roll adapter
classes for View.OnClickListener, DialogInterface.OnClickListener,
MediaPlayer.OnCompletionListener and so on.
This patch auto-tags any .jar-imported interface as @:functionalInterface
when it has exactly one abstract instance method, excluding:
matched by name+arity since their signatures on j.l.Object are
unambiguous — JLS §9.8 excludes these from the SAM count).
The explicit
@FunctionalInterfaceannotation path is preserved.overloadResolution.ml's unify_cf used strict Type.unify to filter
candidates before the cast layer could fire. A function value passed to
an overload whose parameter is a SAM class would be discarded at the
filter step. Now, when strict unify fails, the filter retries via the
SAM conversion path — unifying the function's TFun against the SAM
method's signature, mirroring what AbstractCast does at cast time. Only
candidates whose SAM signature actually matches the function shape
survive, so this isn't a blind accept.
Two supporting fixes round this out:
overloads.ml rate_conv: added a TInst(SAM), TFun case so specificity
ranking works when two SAM-parametered overloads both accept the
same function. The rate descends through the SAM method's signature
rather than raising Not_found, which previously discarded the
candidate during reduce_compatible.
tOther.ml get_singular_interface_field and the parallel walker in
genjvm.ml's check_functional_interface now exclude equals/hashCode/
toString by name+arity, matching the new exclusion in javaModern so
interfaces like Comparator (abstract equals re-declared) — and the
Android SDK's various 'override equals' interfaces — are still
detected as SAMs at both typer and codegen stages.
Tests:
overload disambiguation path with two @:functionalInterface
interfaces of disjoint shape (Int,Int -> Int vs String -> String).
interface shapes — plain SAM, abstract-equals re-declaration,
default/static method skipping, multi-abstract non-SAM, generic-arg
SAM, and overloaded call sites — none annotated @FunctionalInterface.
Runs the generated jar end-to-end to verify both type-checking and
runtime closure-adapter dispatch.
Limitations matching javac:
unsupported; javac doesn't allow lambda conversion to those either.
the same function value remain ambiguous and require explicit
disambiguation.