lenscas / tealr

A wrapper around mlua to generate documentation, definition files and other helpers
69 stars 9 forks source link

Fix building on 32bit systems #78

Closed barsoosayque closed 1 year ago

barsoosayque commented 1 year ago

I was cross-compiling my project from 64bit Linux to 32bit Windows, and one of the impl macros were failing. Turns out, Lua's internal Integer type is different for 32bit and 64bit systems, and try_into depends on that.

Original error:

error[E0271]: type mismatch resolving `<i32 as TryFrom<i32>>::Error == TryFromIntError`
   --> /path/to/code/tealr-0.9.0-alpha4/src/mlu/picker_macro.rs:150:40
    |
150 |                     $conv => $bound_on.try_into().map_err(|x: $error| {
    |                                        ^^^^^^^^ expected enum `Infallible`, found struct `TryFromIntError`
...
290 | impl_from_exact!(i32, mlua::Value::Integer(x), x, TryFromIntError);
    | ------------------------------------------------------------------ in this macro invocation
    |
    = note: this error originates in the macro `impl_from_exact` (in Nightly builds, run with -Z macro-backtrace for more info)
lenscas commented 1 year ago

Thanks for the PR, changes look easy and good.

My only concern is that I currently have no way to test it on 32bit os/hardware, so right now I feel like 32bit support will be very easy to accidentally break again.

I'm still going to merge this as is but I don't think I can "officially" support 32bit until I have a way to properly automatically run tests with it. I am however still happy to accept future PR's to make it work on 32bit if other/new issues arise either way.

bhavyakukkar commented 4 months ago

The changes in this commit (94fb330) were only necessary for lua51 & lua52 as evident from this mlua commit which determines the size of lua_Integer based on the target's pointer-width, for lua51 & lua52 only.

However, lua53 & lua54 had lua_Integer be i64 regardless of target's pointer-width, both at the time of this commit and right now (53 and 54), and fail to compile to 32-bit targets with the following error:

$ cargo run --example mlua_manual -F mlua_lua54,mlua_vendored --target=i686-unknown-linux-gnu
warning: /home/bhavya/dev/rust/learn/crates/tealr/Cargo.toml: `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `bstr` dependency)
warning: /home/bhavya/dev/rust/learn/crates/tealr/Cargo.toml: `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `mlua` dependency)
warning: /home/bhavya/dev/rust/learn/crates/tealr/Cargo.toml: `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `rlua` dependency)
   Compiling mlua-sys v0.4.0
   Compiling mlua v0.9.2
   Compiling tealr v0.9.1 (/home/bhavya/dev/rust/learn/crates/tealr)
error[E0277]: the trait bound `i32: From<i64>` is not satisfied
   --> src/mlu/picker_macro.rs:112:43
    |
112 |                     $conv => Ok($bound_on.into()),
    |                                           ^^^^ the trait `From<i64>` is not implemented for `i32`, which is required by `i64: Into<_>`
...
272 | impl_from_exact_non_failing!(i32, mlua::Value::Integer(x), x);
    | ------------------------------------------------------------- in this macro invocation
    |
    = help: the following other types implement trait `From<T>`:
              <i32 as From<bool>>
              <i32 as From<i16>>
              <i32 as From<i8>>
              <i32 as From<u16>>
              <i32 as From<u8>>
    = note: required for `i64` to implement `Into<i32>`
    = note: this error originates in the macro `impl_from_exact_non_failing` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `tealr` (lib) due to 1 previous error

The above is the output with lua54, but you get the same output with lua53, and both at the time of this commit and now.

As for other lua versions:

I cannot speak for rlua as I have not tested anything for it.

lenscas commented 3 months ago

Sorry for the late response. Was busy with stuff.

Honestly, I'm not too interested in supporting 32bit for tealr. As already mentioned before, I have no use for it myself nor even a way to test it. So, I simply can not promise that it works and will keep working.

I still happily accept PR's that fix 32 bit support, I just can not and thus will not make any promises about it.

bhavyakukkar commented 3 months ago

There's a large scope for supporting lua on wasm (popular wasm targets are all 32-bit) and mlua even supports the wasm32-unknown-emscripten target so it'd still be nice to keep tealr updated on that.

I'm also in the process of testing for 32-bit non-wasm archs, I actually already have changes to make tealr build on 32-bit but want to test them on non-vendored lua before opening a PR (vendored lua successfully builds).

These changes btw recently successfully built my project that uses tealr on wasm32-unknown-emscripten but I'm yet to test and will do soon.

lenscas commented 3 months ago

For wasm: i am actually slowly working on making a Piccolo backend for tealr. Considering that Piccolo is pure rust, it should work nicely with wasm.

Also, last time i looked at mlua and wasm it was just an endless sea of pain. Good to hear that that has improved. But, without a way to test it in CI i still really can't promise that it will keep working.

I do however happily accept a pr to add it to ci. I don't have the time to make that myself however.