--jvm option creates invalid library for android

[acarioni]

Environment macos monterey 12.1 haxe 4.2.4 haxe-concurrent 3.0.2 android studio 2021.1.1

I’ve just discovered an issue regarding the output of the option --jvm in the Android environment. The issue is pretty severe because nowadays Android is a big part of the Java world.

The error can be reproduced in the following way (the error involves the library haxe-concurrent, but I don’t think that the problem is the library per se)

1. create the module below

//Main.hx
package foo;

class Main {
  static var ex = hx.concurrent.executor.Executor.create(1);
  
  public static function main() {
    ex.submit(() -> trace("hello"));
  }
}

2. compile it with the command line haxe foo.Main --library haxe-concurrent --jvm bin/java/main.jar

3. create an empty android project

4. add the dependency implementation files('<PATH TO JAR>') in build.gradle

5. in android studio run the menu command Build>Rebuild Project

The reported error is Type hx.concurrent.executor.Executor$Closure_new_0 is defined multiple times: /Users/foo/temp/bin/java/main.jar:hx/concurrent/executor/Executor$Closure_new_0.class, /Users/foo/temp/bin/java/main.jar:hx/concurrent/executor/Executor$Closure_new_0.class

Besides this error, there are a lot of warnings such as

Invalid signature 'I' for field __hx_toString_depth.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^

On the other hand, if the same module is built using the option --java, there are neither errors nor warnings.

[Simn]

I don't have android studio to test this, but jclasslib says that hx.concurrent.executor.Executor$Closure_new_0 only exists once. Although I don't know if it somehow filters duplicates.

I also don't understand why I would be an invalid signature.

[Simn]

Actually I have an idea why this could happen. Could you try if this reproduces the issue:

class Parent {
	public function new() {}
}

class Child extends Parent {
	public var onResult = function() {
		trace("Hello");
	}
}

function main() {
	new Child().onResult();
}

[acarioni]

Yes! It triggers the same warnings and errors.

The error is

Type foo.Child$Closure_new_0 is defined multiple times: /Users/foobar/temp/bin/android/Parent.jar:foo/Child$Closure_new_0.class, /Users/foobar/temp/bin/android/Parent.jar:foo/Child$Closure_new_0.class

Duplicate class foo.Child$Closure_new_0 found in modules Parent (Parent.jar) and Parent (Parent.jar)

Go to the documentation to learn how to [Fix dependency resolution errors](https://developer.android.com/studio/build/dependencies#resolution_errors).

And the warnings are

Invalid signature 'I' for field __hx_toString_depth.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^
Invalid signature 'I' for field __hx_defaultCapacity.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^
Invalid signature 'I' for field length.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^
Invalid signature 'I' for field current.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^
Invalid signature 'Z' for field isStatic.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
Z
 ^
Invalid signature 'I' for field __hx_toString_depth.
Signature is ignored and will not be present in the output.
Parser error: Expected L, [ or T at position 2
I
 ^

[Simn]

I found a way to test this by using overloads, which I think should be the exact same issue:

class Test {
	overload static public function test() {
		function x() {
			trace("test()");
		}
		x();
	}

	overload static public function test(i:Int) {
		function x() {
			trace("test(I)");
		}
		x();
	}
}

function main() {
	Test.test(); // test(I)
	Test.test(1); // test(I)
}

[Simn]

See if that helped. Nightly builds are available (at some point) at https://build.haxe.org/builds/haxe/.

The fix avoids the problem, but it's not perfect for the original issue. This is (I think) the only situation in which we process the same expression multiple times in the generator, which means that in the case of closures, we end up duplicating the closure.

It would be preferable to not duplicate anything, but a big issue is that we cannot easily outsource this kind of initialization code to a separate method, because then we cannot call that method before calling the parent constructor (because this is uninitialized at that point).

I also still want to find out wtf is wrong with the signatures. I was under the impression that these worked just like descriptors when it comes to basic types, but apparently there's something else going on. Maybe we can just omit field descriptors entirely without losing anything.

[acarioni]

Thank you. I'll try and I'll let you know.

[acarioni]

Hello Simon,

The patch works very well and fixes the error. However the warnings are still there.

[Simn]

I've pushed another change for that. It seems strange to me that fields are special here and generating signatures for basic types would cause warnings, but that's indeed what the specification says.

[Simn]

I'm assuming that this has been addressed. Please let me know if there's still a problem!

[acarioni]

The error and the previous warnings have disappeared, but now there are two new warnings. Apart from them, the generated output works fine in Android.

Invalid stack map table at 172: ifeq L?, error: Source locals {0:Initialized(utest.Runner),1:Initialized(utest.ITest),2:Initialized(haxe.root.EReg),3:Initialized(java.lang.String),4:Initialized(haxe.root.Array),5:Initialized(java.lang.Object),6:Initialized(utest.ITest)} have different local indices than {0:Initialized(utest.Runner),1:Initialized(utest.ITest),2:Initialized(haxe.root.EReg),3:Initialized(java.lang.String),4:Initialized(haxe.root.Array),5:Initialized(java.lang.Object),6:Initialized(utest.ITest),7:top}.

Invalid stack map table at 19: ifnonnull L?, error: Source locals {0:Initialized(java.lang.Object),1:Initialized(java.lang.Object),2:Initialized(java.lang.Boolean),3:Initialized(java.lang.String),4:Initialized(java.lang.Double),5:Initialized(java.lang.Object)} have different local indices than {0:Initialized(java.lang.Object),1:Initialized(java.lang.Object),2:Initialized(java.lang.Boolean),3:Initialized(java.lang.String),4:Initialized(java.lang.Double),5:Initialized(java.lang.Object),6:top}.

[Simn]

Hmm, interesting. I suppose I'll have to find a way to reproduce that without installing Android Studio. I wonder why the normal verifier doesn't catch this.

[Simn]

Do you know if this is still a problem?

[acarioni]

No, I haven't noticed this kind of errors/warnings in a very long time.

[Simn]

Nice, thanks!