Skip to content

[lua] Fix setting dynamic method to null - #13160

Open
tobil4sk wants to merge 3 commits into
HaxeFoundation:developmentfrom
tobil4sk:fix/lua-dynamic-method-null
Open

tobil4sk wants to merge 3 commits into
HaxeFoundation:developmentfrom
tobil4sk:fix/lua-dynamic-method-null

Conversation

@tobil4sk

@tobil4sk tobil4sk commented Nov 7, 2026

Copy link
Copy Markdown
Member

Previously, setting a dynamic method to null would add it to the __fields__ table and set the method field to nil on the object:

local function _hx_obj_newindex(t,k,v)
t.__fields__[k] = true
rawset(t,k,v)
end

This meant that accessing the method again would fall back to using __index, which would use the original prototype implementation of the method:

__index = prototype,

This patch introduces _hx_index_dyn, an __index meta method. Instead of just using the prototype indiscriminately, this function checks whether the method exists in o.__fields__, and only falls back to the prototype if it does not.

The extra function call adds overhead, so it is restricted to classes that contain a dynamic method either in the class itself or in a parent.

@Simn

Simn commented Nov 7, 2026

Copy link
Copy Markdown
Member

This conflicted.

I'm also trying to understand why this is necessary. Generators usually treat a dynamic function as a var field:() -> Void which is initialized in the constructor. It's a special case that has to be considered in some places, but why is the run-time one of those here?

Previously, setting a dynamic method to null would add it to the
__fields__ table and set the method field to nil on the object. This
meant that accessing the method again would fall back to using __index,
which would use the original prototype implementation of the method.

This patch introduces _hx_index_dyn, an __index meta method. Instead of
just using the prototype indiscriminately, this function checks whether
the method exists in o.__fields__, and only falls back to the prototype
if it does not.

The extra function call adds overhead, so it is restricted to classes
that contain a dynamic method either in the class itself or in a parent.
Covers when the dynamic method exists on the current class, but also on
the parent or the parent's parent.
@tobil4sk
tobil4sk force-pushed the fix/lua-dynamic-method-null branch from 97842a5 to 17855fb Compare November 7, 2026 10:39
@tobil4sk

tobil4sk commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

Generators usually treat a dynamic function as a var field:() -> Void which is initialized in the constructor.

That approach sounds better, as it would avoid turning __index into a function that does two table lookups, but I'm guessing it requires some additional scaffolding to not break anything?

I will try to look into it. I see php is generating:

	public $test;

	/**
	 * @return void
	 */
	public function __construct () {
		if ($this->test === null) $this->test = new HxClosure($this, '__hx__default__test');
	}

	public function test ()
	{
		return call_user_func_array($this->test, func_get_args());
	}
	public function __hx__default__test ()
	{
	}

It looks like php still needs the test method to actually exist on the class, and to reroute the field version, is that for reflection or something? Is this the usual approach?

@Simn

Simn commented Nov 7, 2026

Copy link
Copy Markdown
Member

It probably depends on the target, that seems like a good way to retain normal method call semantics while deferring the actual implementation to another implementation.

@tobil4sk

tobil4sk commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

I think it can work on lua with keeping the physical method, but it's simpler if it can be avoided. I came up with an example where not having it would affect behaviour:

class Test {
  public function new() {}
  public dynamic function test() {}
}

class Test2 extends Test {
  public final f:Void -> Void;
  public function new() {
    f = test;
    super();
  }
}

function main() {
  final test = new Test2();
  test.f();
}

If the method doesn't exist on the prototype, test is unset before the super call so f is set to null. However, this example is broken already on php and flash. Should this be specified?

@Simn

Simn commented Nov 7, 2026

Copy link
Copy Markdown
Member

Hmm, that example looks a bit rough. I'll have Claude check how it behaves on all targets at the moment.

Simn commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

I tested @tobil4sk's example (reading an inherited dynamic method before super()) on current development (981c97c), and on this PR for Lua. The test prints whether f is null and then calls it:

function main() {
	final test = new Test2();
	Sys.println("f == null: " + (test.f == null));
	try test.f() catch (e:Dynamic) Sys.println("calling f threw: " + e);
}
Target f = test before super()
eval (--interp) ✅ works
JS (node) ✅ works
Lua (development) ✅ works
Lua (this PR, 17855fb) ✅ works (test is still on Test.prototype, and _hx_index_dyn falls back to it)
Neko ✅ works
Python ✅ works
JVM ✅ works
C++ ✅ works
HL (VM, HashLink master ae0e224) and HL/C ✅ works
PHP ❌ f is null, so test.f() throws Value of type null is not callable
Flash ❓ not run. From genswf9.ml I'd expect null, which matches what @tobil4sk reported

Edit: an earlier version of this comment said HL was broken, based only on reading genhl.ml. I've since run it, and it works on both the VM and HL/C.

Why PHP fails: PHP stores a dynamic method in an instance property, so reading test as a value compiles to reading the property: $this->f = $this->test;. The property has no default value. Only the constructor of the declaring class fills it: if ($this->test === null) $this->test = new HxClosure($this, '__hx__default__test'); in Test::__construct. Test2::__construct reads the property before calling parent::__construct(), so it's still null. The physical test() method doesn't help, because a value read never goes through it.

Why HL works: HL also stores the method in an instance field, and Test2_new reads r0->test before calling Test_new. The constructor-time initialization in genhl.ml (around line 3605) isn't what fills the field, though. genhl also registers the default implementation of every dynamic method as a binding of the class (p.pbindings). The runtime then fills all bindings, inherited ones included, when it allocates the object (hl_alloc_obj in HashLink's src/gc.c), so the field is set before any constructor code runs.

Flash: genswf9.ml (around line 1920) only initializes dynamic methods in the constructor of the declaring class, with HFindProp/HGetProp/HInitProp over c.cl_fields. I didn't find an allocation-time fallback like HL's, but I couldn't run it.

Override variant: I also tried it with override dynamic function test() in Test2. It works on every target I could run, PHP included, because the override is then Test2's own field and its constructor sets it before the body runs. So only an inherited, non-overridden dynamic method read before super() hits this.

What this means for the PR: storing the method in an instance field works if the field is filled when the object is allocated, as HL's bindings do, or if the default implementation stays reachable from the class, as the prototype makes it on JS and Lua. It fails on PHP, where the field is only filled in the declaring constructor. If Lua moves to a field initialized in the constructor, it needs one of those two mechanisms to keep this example working. Whether this behaviour is specified is still open. If it is, PHP (and probably Flash) needs fixing.


Generated by Claude Code

@tobil4sk

tobil4sk commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

It works ok for me on hl when I run it locally. I opened a bug report for flash and php: #13162. I will keep it working here on lua then.

@Simn

Simn commented Nov 7, 2026

Copy link
Copy Markdown
Member

Why I got it wrong: HL does store the method in an instance field, and Test2_new reads r0->test before calling Test_new, just like PHP. But I only looked at the constructor-time initialization in genhl.ml, and that’s not what fills the field. genhl also registers the default implementation of each dynamic method as a binding of the class (p.pbindings). HashLink’s hl_alloc_obj in src/gc.c fills every binding, inherited ones included, when it allocates the object. So the field already holds a closure before any constructor code runs. PHP has nothing like that, which is why it gets null.

@tobil4sk

tobil4sk commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

Hm, I tried to implement it that way:

Test.super = function(self)
    self.test = self.test or Test._hx_default_test
end
-- ...
Test.prototype._hx_default_test = function (self) end
Test.prototype.test = function(self) return rawget(self, "test")(self) end -- use rawget to bypass method resolution

but that creates a problem, as long as the method exists, this will still give a non-null closure:

t.test = null;
final f = t.test; // calling this will give a null error, but it's not null itself

Maybe it could be made to work with some hacking on _hx_bind so it's aware about dynamic methods and returns null here, but it seems complicated and I'm not sure that's a very good idea.

So the two options I can see are:

  • Have the physical method, but intercept __index access to filter out dynamic methods set to null. This is the current version and it adds overhead to method calls on classes using dynamic
  • Remove the physical method to avoid changing __index, but that would break parent dynamic method access before super

Not sure if there are some other ways to get it to work. The fundamental problem here is that on lua there is no distinction between a field set to nil vs an uninitialised field.

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

Successfully merging this pull request may close these issues.

2 participants