-
Notifications
You must be signed in to change notification settings - Fork 1.4k
threads: add feature flags #10206
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
threads: add feature flags #10206
Conversation
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
To modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
What do you think about using the preexisting |
Oh and do directly answer this:
I wrote up some basic thoughts here but the tl;dr; IMO is "anything that requires a runtime dependency" aka something outside of libcore. In that sense the current (or do you have specific things you're curious about too?) |
This adds both the Cargo-level and CLI-level flags for the shared-everything-threads proposal.
As recommended in a review, we can probably use the `threads` feature flag instead for the same kind of conditional compilation.
Since we don't expect users to be interacting with shared-everything-threads modules from the command line, hide this for now.
5701dd0
to
04c70f8
Compare
The latest commits remove the CLI flags since we probably don't want to expose that yet to users and adopt @alexcrichton's suggestion to use the |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:config"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
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.
Technically this won't actually work if the test actually used shared-everything-threads in the sense that this block needs to be updated for fuzzing to propagate the need of shared-everything-threads to wasmtime::Config
, and that's going through CLI flags which means this'll also have to add -Wshared-everything-threads
.
That being said such a change is a superset of what you've got here, and this seems fine otherwise.
This adds CLI-level flags for `shared-everything-threads` based on this [comment]. [comment]: bytecodealliance#10206 (review)
This adds CLI-level flags for `shared-everything-threads` based on this [comment]. [comment]: #10206 (review)
This change adds the minimal, internal flags necessary for testing the shared-everything-threads proposal. It chooses to skip the user-facing CLI flags for now and, as suggested, keeps the Cargo
threads
feature as the sole feature flag gating compilation of threads-related code, which will eventually includeshared-everything-threads
.