search

kyren / piccolo

An experimental stackless Lua VM implemented in pure Rust
Creative Commons Zero v1.0 Universal
1.62k stars 59 forks source link

Respect `__len` and `__index` metamethods in table.unpack #73

Closed Aeledfyr closed 2 months ago

Aeledfyr commented 2 months ago

As of Lua 5.3, the table library respects metamethods; this adds a case to the table.unpack implementation that checks for the existence of those metamethods and falls back to a less efficient implementation that can handles them.

I'm reasonably confident that this implementation matches the behavior of PUC-Rio Lua (5.4.6).

There is one disabled test case that does not match the behavior of PUC-Rio Lua: https://github.com/kyren/piccolo/blob/a75212013e7b4856d67530c1e1adcd53fe498cbc/tests/scripts/unpack_indirect.lua#L67-L80 However, that is due to spec-allowed differences between piccolo and PUC-Rio in the length operator for non-sequences. (PUC-Rio appears to use the last border, piccolo uses the first.)

Aeledfyr commented 2 months ago

The main thing I wasn't certain about in this design is the internal Vec; it would be nice to be able to accumulate the results in-place, but couldn't figure out how to make the sequence callback run with a sub stack, rather than treating the entire stack as arguments/return values.

The Sequence API has been updated to allow for preserving the stack between calls; the implementation now uses that instead of an internal buffer.

kyren commented 2 months ago

This is really good! I have some very minor nits, but they're extremely minor, and I'm gonna go ahead and merge this because it's a strict improvement (as we discussed in chat).