This is a somewhat involved change with two breaking API changes:
1) Lua::coerce_xxx methods now return Option (this is easier and faster than
dealing with Result)
2) rlua numeric conversions now allow more loss of precision
conversions (e.g. 1.5f32 to 1i32)
The logic for the first breaking change is that mostly the coerce methods are
probably used internally, and they make sense as low-level fallible casts and
are now used as such, and there's no reason to confuse things with a Result with
a large error type and force the user to match on the error which will hopefully
only be FromLuaConversionError anyway.
The logic for the second change is that it matches the behavior of
num_traits::cast, and is more consistent in that *some* loss of precision
conversions were previously allowed (e.g. f64 to f32).
The problem is that now, Lua::coerce_integer and Lua::unpack::<i64> have
different behavior when given, for example, the number 1.5. I still think this
is the best option, though, because the Lua::coerce_xxx methods represent how
Lua works internally and the standard C API cast functions that Lua provides,
and the ToLua / FromLua code represents the most common form of fallible Rust
numeric conversion.
I could revert this change and turn `Lua::eval::<i64>("1.5", None)` back into an
error, but it seems inconsistent to allow f64 -> f32 loss of precision but not
f64 -> i64 loss of precision.
Tried to explain the rationale for safety around callbacks in Lua and Scope a
bit better, because every time I don't look at this for a while I forget my
reasoning. I'm not always so great at using the right terminology, so to
whoever reads this, if I got this wrong please tell me.
Uses the same UserData trait, and should at least in theory support everything
that 'static UserData does, except that any functions added that rely on
AnyUserData are pretty much useless.
Probably pretty slow and I'm not sure how to make it dramatically faster, which
is a shame because generally when you need non'-static userdata you might be
creating it kind of a lot (if it was long-lived, it would probably be 'static).
Haven't added tests yet, will do that next.
Callbacks should not be able to capture their arguments and hold onto them,
because the `&Lua` used in previous calls will not remain valid across calls.
One could imagine an API where the specific `&Lua` is simply stored inside the
`Scope` itself, but this is harder to do, and would (badly) encourage storing
references inside Lua userdata.
Ideally, the only way it should be possible to store Lua handles inside Lua
itself is through usafety or the `rental` crate or other self-borrowing
techniques to make references into 'static types. If at all possible this
roadblock should stay, because reference types inside userdata are almost always
going to lead to a a memory leak, and if you accept the risks you should just
use `RegistryKey` with its manual removal.
The documentation describing it being a logic bug to access "outer" callback
handles when inside an "inner" callback is inaccurate, that was only true when
using an older design for handle values.
Also, there is no reason to have a separate 'callback lifetime, because 'scope
is already invariant and just using 'scope seems equivalent.
I'm a bit unclear on whether bumping a dependency's minor version is considered
a semver breaking change, but without doing this everyone probably gets
deprecation warnings?
Vastly simpler and less magical than using a fixed size magical section of the
active stack, and seems to be no slower. The only real downside is that
it *seems* extremely extremely hacky (and to be fair, it is).
This should protect against being able to trigger a stack assert in Lua. Lua
and associated types shoul be able to assume that LUA_MINSTACK stack slots are
available on any user entry point. In the future, we could turn check_stack
into something that only checked the Lua stack when debug_assertions is true.
Since we now optionally use stack spaces for handle values, we have to be
mindful of whether our stack handle points to the stack in an outer level of
Lua "stack protection". We now keep track of the "recursion level" of Lua
instances, and do not allow ref manipulation on "outer" Lua instances until the
inner callback has returned. Also, update the documentation to reflect the
additional panic behavior.
Also, don't bother asserting if the userdata has no metatable, just behave as
though the userdata has no type. This should be impossible to trigger currently
without the debug library, but it is not really that useful of an assert anyway.
Also makes `Lua` and associated types !UnwindSafe and !RefUnwindSafe, which they
should be because they are intensely internally mutable. Lua IS still panic
safe, but that doesn't mean it should be marked as UnwindSafe (as I understand
it).
Okay, so this is kind of a mega-commit of a lot of performance related changes
to rlua, some of which are pretty complicated.
There are some small improvements here and there, but most of the benefits of
this change are from a few big changes. The simplest big change is that there
is now `protect_lua` as well as `protect_lua_call`, which allows skipping a
lightuserdata parameter and some stack manipulation in some cases. Second
simplest is the change to use Vec instead of VecDeque for MultiValue, and to
have MultiValue be used as a sort of "backwards-only" Vec so that ToLuaMulti /
FromLuaMulti still work correctly.
The most complex change, though, is a change to the way LuaRef works, so that
LuaRef can optionally point into the Lua stack instead of only registry values.
At state creation a set number of stack slots is reserved for the first N LuaRef
types (currently 16), and space for these are also allocated separately
allocated at callback time. There is a huge breaking change here, which is that
now any LuaRef types MUST only be used with the Lua on which they were created,
and CANNOT be used with any other Lua callback instance. This mostly will
affect people using LuaRef types from inside a scope callback, but hopefully in
those cases `Function::bind` will be a suitable replacement. On the plus side,
the rules for LuaRef types are easier to state now.
There is probably more easy-ish perf on the table here, but here's the
preliminary results, based on my very limited benchmarks:
create table time: [314.13 ns 315.71 ns 317.44 ns]
change: [-36.154% -35.670% -35.205%] (p = 0.00 < 0.05)
create array 10 time: [2.9731 us 2.9816 us 2.9901 us]
change: [-16.996% -16.600% -16.196%] (p = 0.00 < 0.05)
Performance has improved.
create string table 10 time: [5.6904 us 5.7164 us 5.7411 us]
change: [-53.536% -53.309% -53.079%] (p = 0.00 < 0.05)
Performance has improved.
call add function 3 10 time: [5.1134 us 5.1222 us 5.1320 us]
change: [-4.1095% -3.6910% -3.1781%] (p = 0.00 < 0.05)
Performance has improved.
call callback add 2 10 time: [5.4408 us 5.4480 us 5.4560 us]
change: [-6.4203% -5.7780% -5.0013%] (p = 0.00 < 0.05)
Performance has improved.
call callback append 10 time: [9.8243 us 9.8410 us 9.8586 us]
change: [-26.937% -26.702% -26.469%] (p = 0.00 < 0.05)
Performance has improved.
create registry 10 time: [3.7005 us 3.7089 us 3.7174 us]
change: [-8.4965% -8.1042% -7.6926%] (p = 0.00 < 0.05)
Performance has improved.
I think that a lot of these benchmarks are too "easy", and most API usage is
going to be more like the 'create string table 10' benchmark, where there are a
lot of handles and tables and strings, so I think that 25%-50% improvement is a
good guess for most use cases.
This will potentially panic on Drop of a `Lua` instance, which may be an abort
if this is a double panic, but that is more desirable than such a bug being
hidden.
Previously, on an internal panic, the Lua stack would be reset before panicking
in an attempt to make sure that such panics would not cause stack leaks or leave
the stack in an unknown state. Now, such panic handling is done in stack_guard
and stack_err_guard instead, and this is for a few reasons:
1) The previous approach did NOT handle user triggered panics that were outside
of `rlua`, such as a panic in a ToLua / FromLua implementation. This is
especially bad since most other panics would be indicative of an internal bug
anyway, so the utility of keeping `rlua` types usable after such panics was
questionable. It is much more sensible to ensure that `rlua` types are
usable after *user generated* panics.
2) Every entry point into `rlua` should be guarded by a stack_guard or
stack_err_guard anyway, so this should restore the Lua stack on exiting back
to user code in all cases.
3) The method of stack restoration no longer *clears* the stack, only resets it
to what it previously was. This allows us, potentially, to keep values at
the beginning of the Lua stack long term and know that panics will not
clobber them. There may be a way of dramatically speeding up ref types by
using a small static area at the beginning of the stack instead of only the
registry, so this may be important.
So, despite staring intently at the params structure magic in protect_lua_call,
there is still a nasty bug. In the event of an error, the return value of the
parameters structure could be dropped despite being mem::unintialized. Of
course, the actual return values are incidentally always Copy I think, so this
wasn't an actual bug, but I've proven to myself the danger of such dark majyyks.
Just use Option and be done with it, it doesn't have to be so complicated!
Also document why there are a slew of random functions in the ffi module.
This simplifies the Scope lifetimes, and should make it a compile error for
scope created handles to exit the scope. This should be strictly better, as you
would never WANT to do this, but I hope that I have not caused a subtle lifetime
problem that would prevent passing those created handles back into Lua. I've
tested every situation I can think of, and it doesn't appear to be an issue, but
I admit that I don't fully understand everything involved and I could be missing
something.
The reason that I needed to do this is that if you can let a scope handle escape
the scope, you have a LuaRef with an unused registry id, and that can lead to
UB. Since not letting the scope references escape is a strict improvement
ANYWAY (if I haven't caused a lifetime issue), this is the easiest fix.
This is technically a breaking change but I think in most cases if you notice it
you would be invoking UB, or you had a function that accepted a Scope or
something. I don't know if it's worth a version bump?
kyren