add wasmbind feature #4875
add wasmbind feature #4875
Conversation
vm/src/stdlib/time.rs
Outdated
| } | ||
| // Date.now returns unix time in milliseconds, we want it in seconds | ||
| Ok(Date::now() / 1000.0) | ||
| panic!("Date not available") |
There was a problem hiding this comment.
If you are compiling wasm32-unknown-unknown and not wasi then std time doesn't work, so there is no way to get a date generally as far as I know for for wasm32-unknown-unknown.
There was a problem hiding this comment.
Adding to @bdemann's reasoning (we're on the same team), in our project, which must compile and run in a wasm32-unknown-unknown environment, std time doesn't work, and we have our own time implementation that we import from the host environment. We would love for a solution to this problem as well, but this PR is scoped for general use.
There was a problem hiding this comment.
When the interpreter panics, the interpreter process comes to a complete emergency stop. From there, you have to start it again. Is this what you want?
There was a problem hiding this comment.
Typically, unsupported platforms in Python just don't expose the module/function/constant etc (the os module is an example of this). If the time module doesn't work for the target it would make more sense to just not have it be available.
Baring that, an exception would be the second best option.
There was a problem hiding this comment.
I also vote for hiding the function time.time when not available.
Then user can set it like time.time = custom_time when not hasattr(time, 'time')
There was a problem hiding this comment.
Ah, I found process_time and process_time_ns raises error. NotImplementedError also will be ok.
vm/src/vm/vm_object.rs
Outdated
| } | ||
| let mut s = String::new(); | ||
| self.write_exception(&mut s, &exc).unwrap(); | ||
| error(&s); | ||
| panic!("{}; exception backtrace above", msg) |
There was a problem hiding this comment.
This will fail to compile if the wasmbind feature is not enabled.
|
oh, no. this one is not merged yet. |
There was a problem hiding this comment.
additionally, rustpython_wasm will require the new feature to work.
I am really sorry for late review.
Since the patch is created from unwritable branch, I will finish it on a new PR #5390
| #[cfg(any( | ||
| not(target_arch = "wasm32"), | ||
| target_os = "wasi", | ||
| not(feature = "wasmbind") | ||
| ))] |
There was a problem hiding this comment.
| #[cfg(any( | |
| not(target_arch = "wasm32"), | |
| target_os = "wasi", | |
| not(feature = "wasmbind") | |
| ))] | |
| #[cfg(not(all( | |
| target_arch = "wasm32", | |
| feature = "wasmbind", | |
| not(target_os = "wasi") | |
| )))] |
The condition is looking confusing, which actually is negation of the below
| @@ -99,7 +103,7 @@ mod time { | |||
| fn now() -> f64; | |||
| } | |||
| // Date.now returns unix time in milliseconds, we want it in seconds | |||
| Ok(Date::now() / 1000.0) | |||
| return Ok(Date::now() / 1000.0); | |||
There was a problem hiding this comment.
| return Ok(Date::now() / 1000.0); | |
| Ok(Date::now() / 1000.0) |
|
Any idea to add a test not to break this again? |
Resolves #4320