-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve rust bindings #513
Conversation
5c6e6bc
to
f21d72c
Compare
f21d72c
to
18b691a
Compare
24bc2cb
to
f3c09ff
Compare
f3c09ff
to
c1924c9
Compare
@@ -37,12 +37,12 @@ let compile ~includes:_ ~opt_lvl:_ debug (files : Fpath.t list) : | |||
|
|||
let rustc_cmd : Cmd.t = | |||
Cmd.( | |||
rustc_bin % "--target=wasm32-unknown-unknown" % "--extern" | |||
rustc_bin % "--target=wasm32-unknown-unknown" % "--edition=2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain to me quickly why the edition=2021
is needed? (This is simply out of curiosity and not blocking in anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's because Arthur broke rust in rust-lang/rust#129321.
and wants to avoid using the broken version.
(jk 😅)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure, suddenly owi rustc
didn't find the owi crate without extern crate
which is something that only existed in edition 2015 of rust and has become optional since rust 2018. I am not sure which it only triggered now, maybe differences between the CI rustc and mine ?
let x = u32_symbol(); | ||
let c = char::from_u32(x).unwrap_or_else(|| stop_exploration()); | ||
c | ||
Symbolic::symbol() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would vote for removing all of them now. It is not as if they were used a lot and we would break any existing workflow... WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, but I thought better safe than sorry, especially if we want old code to work as is easily
Thanks! |
No description provided.