-
Notifications
You must be signed in to change notification settings - Fork 835
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
chore: fix all warnings #5363
base: main
Are you sure you want to change the base?
chore: fix all warnings #5363
Conversation
This fixes all compiler warnings. Note that `__cbindgen_hack__ = "yes"` is a bit weird - there is no need to make it support a value -- presence of a keyword is an indicator enough. Also, I couldn't figure out what actually sets that value.
@@ -102,6 +102,7 @@ wasmer-artifact-create = ["wasmer-compiler/wasmer-artifact-create"] | |||
static-artifact-load = ["wasmer-compiler/static-artifact-load"] | |||
static-artifact-create = ["wasmer-compiler/static-artifact-create"] | |||
webc_runner = ["virtual-fs", "webc"] | |||
dylib = [] |
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.
This feature was deprecated long ago. We shouldn't add it back
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.
You use it in
wasmer/lib/c-api/src/wasm_c_api/engine.rs
Line 258 in d431db9
#[cfg(all(feature = "compiler", any(feature = "compiler", feature = "dylib")))] |
dylib
is not declared. Should it be removed there?
llvm = [] | ||
run = [] |
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.
This create will be deprecated (removed from the repo) very soon, we shouldn't add new features to it
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.
If the features are used in code, they should be declared in cargo.toml - otherwise its like using a variable without declaring it first - ok for some languages, but usually a bad form for Rust :) Should they be removed there, or should these be added here until then?
|
||
[features] | ||
default = [] | ||
tracing = [] |
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.
Why is tracing a feature if it's not used anywhere?
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.
it is used in
#[cfg(feature = "tracing")] |
#[cfg(feature = "tracing")] |
@syrusakbary thx for the quick review! The reason I added those is because you have a lot of |
This fixes all compiler warnings. Note that
__cbindgen_hack__ = "yes"
is a bit weird - there is no need to make it support a value -- presence of a keyword is an indicator enough. Also, I couldn't figure out what actually sets that value.