Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] Generalization of @FunctionalInterface #11014

Closed
EliteMasterEric opened this issue Mar 14, 2023 · 9 comments
Closed

[java] Generalization of @FunctionalInterface #11014

EliteMasterEric opened this issue Mar 14, 2023 · 9 comments
Milestone

Comments

@EliteMasterEric
Copy link
Contributor

EliteMasterEric commented Mar 14, 2023

I am attempting to extern to a function which registers an event with an event handler. See below.

// Modified from dumped externs for clarity, the originals use more type parameters

extern class ItemGroupEvent {
  function register(param1:ItemGroupEvent_ModifyEntries):Void;
}

// In Java this has @FunctionalInterface
extern interface ItemGroupEvents_ModifyEntries {
	function modifyEntries(param1:ItemGroupEntries):Void;
}

When I try to call register from my Haxe code, I get:

// Again, modified to make types easier to understand.
Could not find a suitable overload, reasons follow
Overload resolution failed for (param1 : net.fabricmc.fabric.api.event.Event.T) -> Void
lines 70-81 : (content:ItemGroupEntries) -> Unknown<0> should be ItemGroupEvents_ModifyEntries
lines 70-81 : ... For function argument 'param1'

The issue is that I want to pass a lambda in, but the Java extern expects an object implementing a functional interface.

I expected that I could resolve the issue by creating my own extern like so:

package java.util.function;

@:native("java.util.function.Supplier")
extern interface JSupplier<T> {
  public function get():T;
}

/**
 * A basic supplier class.
 * TODO: The generated Java instantiates and uses this but ideally the actual function should be passed instead.
 */
@:dce
class BaseSupplier<T> implements JSupplier<T> {
  public var fun:()->T;

  public function new(fun:()->T) {
    this.fun = fun;
  }

  public function get():T {
    return fun();
  }
}

/**
 * A supplier is simply an interface for a lambda function
 * which takes no arguments and returns a value.
 * 
 * Useful for lazy evaluation.
 */
@:dce
@:forward
abstract Supplier<T>(BaseSupplier<T>) {
  // Use an abstract so that functions which take a Supplier<T> can take a ()->T as well.

  public function new(fun:()->T) {
    this = new BaseSupplier(fun);
  }

  @:from
  public static function fromLambda<T>(fun:()->T):Supplier<T> {
    return new Supplier<T>(fun);
  }
}

This works in basic Haxe code, however I end up with a new issue: JARs which I import with --java-lib-extern break when I define an abstract where Java expects an interface to be.

./libs/guava-31.1-jre.jar@com/google/common/base/Supplier.class:1: character 1 : Should extend by using a class

I also considered renaming my abstract, but this would require creating and maintaining modified externs for ItemGroupEvent, and if I later import a JAR which expects an unmodified ItemGroupEvent, the problem would continue to expand.

I believe the only solution here is to modify Haxe to properly interpret functional interfaces from Java code. As mentioned in this blog post and seen in this ML code, Haxe implements automatic conversion for:

  • java.lang.Runnable <-> () -> Void
  • java.util.function.Consumer <-> (value:T) -> Void
  • java.util.function.BiConsumer <-> (value:T, value2:U) -> Void
  • java.util.function.Function <-> (value:T) -> R

And notably, Consumer works exactly as intended in connect-haxe-sdk (the second argument of this function is a java.util.function.Consumer<Flow>).

However, this only works for these SPECIFIC interfaces, and does not work for any other interfaces with @FunctionalInterface specified, including java.util.function.Supplier, ItemGroupEvents_ModifyEntries from the library I'm using, and [countless others](language:java @FunctionalInterface).

JavaFunctionalInterfaces should determine each functional interface's name, classpath, parameters, arguments, and return type from reading the Java classpath properly.

EDIT: For the sake of tracking, here is a list of relevant issues:

@Simn
Copy link
Member

Simn commented Mar 14, 2023

Urgh, not this again... I've made en embarrassing amount of attempts at implementing this but always ran into one problem or another. The most recent approach was this branch (which I have just updated): https://github.com/HaxeFoundation/haxe/tree/jvm_functional_interfaces_for_real

Unfortunately, I don't remember what problem I ran into there. The branch doesn't add any tests, so there's a good chance that it simply doesn't work at all.

@Simn
Copy link
Member

Simn commented Mar 14, 2023

By the way, are you sure that connect-haxe-sdk example works? I don't see how the typer would allow assigning a function type to an interface. That's actually one of the common problems we still have to address because IIRC this only works with awkward abstract workarounds.

@EliteMasterEric
Copy link
Contributor Author

EliteMasterEric commented Mar 14, 2023

The branch doesn't add any tests, so there's a good chance that it simply doesn't work at all.

I have confirmed the Haxe branch does not work, I created a test case on my branch. Add and subtract work but multiply and divide throw compilation errors, stating that they got (a:Int, b:Int) -> Int when test.MathOperation (a functional interface which should match that signature) was expected.

Following up on connect-haxe-sdk, all test cases seem to run fine on the Java target. That's only because it uses a java.util.function.Consumer, which has been manually defined as equivalent to a lambda in OCaml.

@Simn
Copy link
Member

Simn commented Mar 15, 2023

I have confirmed the Haxe branch does not work, I created a test case on my branch. Add and subtract work but multiply and divide throw compilation errors, stating that they got (a:Int, b:Int) -> Int when test.MathOperation (a functional interface which should match that signature) was expected.

Yes that's what I'm saying, Haxe doesn't admit the assignment in its own unification. What I'm after on that branch is to get the generator side working. I think using cast multiply as an argument should make it work because it shuts up the typer and allows the code through to the generator.

Following up on connect-haxe-sdk, all test cases seem to run fine on the Java target. That's only because it uses a java.util.function.Consumer, which has been manually defined as equivalent to a lambda in OCaml.

Are you sure these tests are actually run? The following does not work for the same reason as your multiply example:

class Flow {
	public function new() {}
}

function test(c:java.util.function.Consumer<Flow>) {}

function main() {
	test(function(f:Flow) {});
}
source/Main.hx:8: characters 7-26 : (f : Flow) -> Void should be java.util.function.Consumer<Flow>
source/Main.hx:8: characters 7-26 : ... For function argument 'c'

The compiler simply doesn't allow the assignment, so unless there's some abstract magic going on this should not work even for the hardcoded types like Consumer.

@EliteMasterEric
Copy link
Contributor Author

Just did a deeper dive to confirm, the compile define that sets typedef StepFunc = connect.native.JavaConsumer<Flow> is in fact not run even when testing on Java.

Trying to test with casting:

result = test.Main.performMathOperation(cast multiply);

Returns:

Exception in thread "main" java.lang.IllegalAccessError: class haxe.root.Run$Run_multiply cannot access its superinterface test.MathOperation (haxe.root.Run$Run_multiply and test.MathOperation are in unnamed module of loader 'app')
        at java.base/java.lang.ClassLoader.defineClass1(Native Method)
        at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1012)
        at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
        at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
        at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
        at haxe.root.Run.main(Run.hx:9)
        at haxe.root.Run.main(Run.hx:1)

@Simn
Copy link
Member

Simn commented Mar 15, 2023

Thank you for checking, I'll look into this and commit something that allows the assignment on the Haxe side so that this easier to test.

@Simn
Copy link
Member

Simn commented Mar 15, 2023

I've made a basic implementation to allow the assignment and also added your test case. I can confirm that this fails the same way:

Exception in thread "main" java.lang.IllegalAccessError: class haxe.root.Run$Run_multiply cannot access its superinterface test.MathOperation (haxe.root.Run$Run_multiply and test.MathOperation are in unnamed module of loader 'app')

I don't really know what this means yet and I don't think it's strictly related to the issue itself, but rather some native interop problem. If you have any insight, please let me know.

A pure Haxe implementation of your code does now compile and run on this branch:

interface MathOperation {
	function perform(a:Int, b:Int):Int;
}

class Ops {
	static public final add:MathOperation = (a, b) -> a + b;
	static public final subtract:MathOperation = (a, b) -> a - b;

	static public function performMathOperation(operation:MathOperation) {
		return operation.perform(8, 4);
	}
}

class Main {
	static function main() {
		var result = Ops.performMathOperation(Ops.add);
		trace('Add: ${result}');

		result = Ops.performMathOperation(Ops.subtract);
		trace('Subtract: ${result}');

		result = Ops.performMathOperation(multiply);
		trace('Multiply: ${result}');

		result = Ops.performMathOperation(function(a, b):Int {
			return Std.int(a / b);
		});
		trace('Divide: ${result}');
	}

	static function multiply(a, b):Int {
		return a * b;
	}
}
source/Main.hx:17: Add: 12
source/Main.hx:20: Subtract: 4
source/Main.hx:23: Multiply: 32
source/Main.hx:28: Divide: 2

@EliteMasterEric
Copy link
Contributor Author

Interesting, does the above "pure Haxe" example work in other target languages too?

There may be an issue with my example, I'm not sure, maybe try rewriting it.

@Simn Simn added this to the Release 4.3 milestone Mar 24, 2023
@Simn
Copy link
Member

Simn commented Mar 24, 2023

Implemented in #11019.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants