Fix the WASM build #701
Fix the WASM build #701
Conversation
fd693ad to
de1bc93
Compare
vm/src/stdlib/json.rs
Outdated
| @@ -188,31 +188,31 @@ impl<'de> Visitor<'de> for PyObjectDeserializer<'de> { | |||
| } | |||
| } | |||
|
|
|||
| pub fn ser_pyobject(vm: &mut VirtualMachine, obj: &PyObjectRef) -> serde_json::Result<String> { | |||
There was a problem hiding this comment.
Please do not expose functions like this. I would prefer to figure out what went wrong, and fix that instead.
There was a problem hiding this comment.
What about as a method on VM? json could be a pub(crate) mod and it wouldn't really be exposed. The code would also be faster because it's not looking up a function and loading it from a module, it's just calling a Rust function.
There was a problem hiding this comment.
I don't really agree with @windelbouwman here - this isn't an implementation detail, it's the actual python function so it's inevitable to expose these at some point to allow developing native extensions without unnecessary overhead.
The code would also be faster because it's not looking up a function and loading it from a module, it's just calling a Rust function.
Yep! 👍
vm/src/stdlib/mod.rs
Outdated
| @@ -1,6 +1,6 @@ | |||
| mod ast; | |||
| mod dis; | |||
| mod json; | |||
| pub mod json; | |||
There was a problem hiding this comment.
Please keep this code private. Having this code unexposed is a good thing, and avoids leaking internals.
|
I am going to merge this just in order to stop the CI builds from failing, but I feel like the VM methods are a good compromise for concerns regarding leaking internals. |
For some reason it was panicking when trying to import
json.{dumps,loads}, so I just switched to using plain rust functions straight fromjson.rs.