kyren / piccolo

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

Unify string conversion logic, remove Value::to_string footgun #80

Closed Aeledfyr closed 2 months ago

Aeledfyr commented 2 months ago

This makes Value no longer directly implement fmt::Display, so it no longer has a to_string method. This is probably a good thing, as calling to_string would silently corrupt non-utf8 data.

Method changes:

Adds a FromValue impl for string::String<'gc>

This significantly reduces the boilerplate for stdlib methods that deal with strings.

kyren commented 2 months ago

I really like this PR!

This changes the semantics of impl<'gc> FromValue<'gc> for String<'gc> {...} to also convert numbers / integers to strings automatically.

This is a good change, because it makes the behavior of FromValue for String mirror the behavior of FromValue for i64 / f64... the numeric FromValue impls convert from a string, so the String FromValue impl should convert from numbers.

I have already begun to regret the auto-conversions to/from String. I know that this is the semantics of Lua itself, but even PUC-Rio Lua has started to back off of it with the eventual goal of only using metatables to support conversion, so that it can be turned off.

I have vague plans to support turning off this kind of auto conversion either by doing the conversion through metatables or through whatever BYOV (Bring Your Own Value) turns into, but these are pretty far off. In the meantime, this is strictly more consistent, and simpler.