[yuxiaomao]

It's a rare case in our codebase, and it doesn't really cause problem at runtime. However it fail hl-check

class Base<T> {
	public dynamic function onItem(item : T) : Void {}
	public function new() {}
}

class Sub extends Base<String> {
	override function onItem(item : String) {}
}

function main() {
	new Sub();
}

Fail hl-check at:

 6 | class Sub extends Base<String> {
 7 |  override function onItem(item : String) {}
 8 | }
   |
   | Check failure at fun@25 @3 - Register 1((String):void) should be (dyn):void and not (String):void

Before fix

	fun@25(19h) (Sub):void
	; src/Main.hx:6 (Sub.new)
		r0 Sub
		r1 (String):void
		r2 void
		.6     @0 getthis 1,[0]
		.6     @1 jnotnull 1,2
		.6     @2 instanceclosure 1, Sub.onItem(0)
		.6     @3 setthis [0],1 ; <- error here, this[0] is field in Base (generic class), with type (dyn):void
		.6     @4 call 2, Base.new(0)
		.6     @5 ret 2

After fix

	fun@25(19h) (Sub):void
	; src/Main.hx:6 (Sub.new)
		r0 Sub
		r1 (dyn):void
		r2 (String):void
		r3 void
		.6     @0 getthis 1,[0]
		.6     @1 jnotnull 1,3
		.6     @2 instanceclosure 2, Sub.onItem(0)
		.6     @3 instanceclosure 1, fun$27(2) ; <- added a level of closure when type can't safe_cast, as if we do a cast_to
		.6     @4 setthis [0],1
		.6     @5 call 3, Base.new(0)
		.6     @6 ret 3
	fun@27(1Bh) ((String):void,dyn):void
	; src/Main.hx:7 (fun$27)
		r0 (String):void
		r1 dyn
		r2 String
		r3 void
		.7     @0 safecast 2,1
		.7     @1 callclosure 3, 0(2)
		.7     @2 ret 3

[yuxiaomao]

With more tests there is

s.onItem = function(item) { trace(item + " !"); }

fail with

[ERROR] src/Main.hx:13: characters 2-3

 13 |  s.onItem = function(item) { trace(item + " !"); }
    |  ^
    | Check failure at fun@32 @3 - Register 2((String):void) should be (dyn):void and not (String):void

So added a new fix (I removed the local optimization that skip nullcheck in cast_to, it's better fit in hlopt but that might introduce other problem so better in another PR).

Cpp, jvm are really failing the tests. Fix suggested by Claude (seems that previously a new field is created for the dynamic function, which isn't good. But I'm not able to understand much of it.)

[Simn]

The JVM fix looks a bit silly, surely this should become part of the if not (has_class_flag c CInterface) && is_physical_field cf then self#generate_field gctx jc c mtype cf condition so it doesn't even call generate_field.

[Simn]

JVM and Eval look good to me now.

@Aidan63 Could you check the CPP change here? The change makes sense to me overall, but I'm not quite sure about the new_default_closure part of it.

[Aidan63]

The new_default_closure change just seems to explicitly place the constructed closure into a callable object. Not strictly required since it's being assigned to a callable already so it happens implicitly, but shouldn't effect anything either.

[yuxiaomao]

Claude's message (short version):

Filtering in filter_dynamic_functions would also drop the override's __default_ closure class and its install in __alloc_dynamic_functions and the constructor. dump_dynamic in cppGen.ml still calls Sub_obj::__alloc_dynamic_functions (it looks at the Haxe fields), so it compiles, lands on Base's inherited static, and silently installs Base's default. The accessor is the only field-bound piece that must go, since the override has no field of its own, so I'd keep the check there with a comment.

The explicit Callable wrap is needed when the signatures differ, as with a generic base: without it MSVC fails with "conversion requires a second user-defined-conversion" (Callable_obj<void(String)>* → Callable<void(Dynamic)>). It's the same wrap hxcpp already emits when assigning a lambda to that field.

[yuxiaomao]

I can't understand any cpp code yet, but I'm letting it try harder with your review in mind*

[Aidan63]

I'm confused by the changes now, I say lets just revert that last change and go with the initial one. I can always look at it later.

[tobil4sk]

It would be good to open an issue about these disabled tests

UPDATE: solved in #13160

[yuxiaomao]

If you would like to please open issues, also note that 13038 has if !cppia too