-
Notifications
You must be signed in to change notification settings - Fork 6
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
WASM compatibility #3
Comments
Hmmm. It's an interesting idea. My concern is that presumably a user of this crate needs it for some kind of important performance characteristic of their code. If that particular crate wanted to opt-out of this functionality in no-std environments, they'd provide that via a feature flag. |
valid points (except that WASM is not no-std). Making the error also more obvious by printing something to the console before it would crash is also not an option (At least, not without opening a can of worms which I agree with it being out of scope for this library) So, except for either printing something to the console at compile time whenever it detects it is being compiled to WASM or a separate trait which can be turned off there is not much that can be done. I guess that this means that this issue can be closed. Unless either of the options left are something you wouldn't mind thinking about. (I don't mind writing either of them and opening a pull request) |
I think at the very least refusing to compile on threadless platforms makes sense. |
That may make the problem worse. Because that would mean an entire crate becomes unusable even if only a very small/niche feature of said crate uses this library. So, maybe compile error by default but with a "I know what I am doing" feature to allow to compile anyway? |
To my mind, the "I know what I'm doing flag" would simply be a "don't use DeferDrop at all" flag. Like, if a depender wants to operate in non-threaded environments, they'd provide a flag to disable the dependency (or, conversely, they'd have a default flag that enables it, because feature flags are additive by design). Again, my concern is that if a library is using DeferDrop, they are presumably relying on its behavior because they have a particular performance need, so I wouldn't want that to silently be disabled just based on the evironment you're in. I'm also concerned about diamond dependency issues- if library A and B both depend on DeferDrop, and B enabled a feature flag that caused DeferDrop to disable itself, it would also be disabled for A, which might not be able to operate that way. The preferable solution would be for B to conditionally depend on DeferDrop, so that A isn't affected. |
with the "I know what I am doing" flag I meant a flag that would stop the error at compile time and thus let it crash on run time if something that implements DeferDrop ever gets dropped, just as what happens right now. That way it is still very obvious that DeferDrop is used and what can happen because of it. However, it also gives WASM users a way to use libraries that rely on DeferDrop but only for a (small) part until a proper solution is implemented. This flag also allows you to use DeferDrop in WASM VM's that do support multi threading. As far as I know, that is something that is planned to be part of WASM. It is just not right now, and it will probably take a good while before it is and even longer for it to be usable on the web. |
I don't think I'm going to add a feature flag that literally turns off the whole library; I'd say that dependents who want to conditionally depend on defer-drop can do so via cargo's optional dependencies feature. However, I am open to the possibility of platform-specific variants that use different "background task spawning" features; presumably WASM has some kind of entry point into the JS event queue. |
in that case I would suggest to look on how to force the user to register a function in wasm rather than making a binding to js yourself. That way both WASM and WASI can keep working and you may be able to step around the wams-bindgen vs std-web vs whatever else there is around there. |
Would it be possible to turn the functionality of this library off? I know it sounds stupid but you can't really create new threads on WASM (crashes at runtime). Despite this, most rust code just works out of the box even if WASM is not their main concern.
I'd hate to see a future where this crate becomes the deciding factor to wether I can or can not use some completely other crate.
It is a cool idea though, and I'm curious to see where it goes.
The text was updated successfully, but these errors were encountered: