Skip to content

Commit 56ee29d

Browse files
authored
Refactor handling of --features on the CLI (#1668)
This commit uses the `bitflags::Flags` trait to refactor the parsing of the `--features` CLI flag to avoid needing to manually list out wasm features. This is something I at least always personally forget during reviews and such so it should make the CLI support more robust.
1 parent 00cc81c commit 56ee29d

9 files changed

+45
-52
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ termcolor = { workspace = true }
115115
# Dependencies of `validate`
116116
wasmparser = { workspace = true, optional = true, features = ['validate'] }
117117
rayon = { workspace = true, optional = true }
118+
bitflags = { workspace = true, optional = true }
118119

119120
# Dependencies of `print`
120121
wasmprinter = { workspace = true }
@@ -201,7 +202,7 @@ default = [
201202
]
202203

203204
# Each subcommand is gated behind a feature and lists the dependencies it needs
204-
validate = ['dep:wasmparser', 'rayon', 'dep:addr2line', 'dep:gimli']
205+
validate = ['dep:wasmparser', 'rayon', 'dep:addr2line', 'dep:gimli', 'dep:bitflags']
205206
print = []
206207
parse = []
207208
smith = ['wasm-smith', 'arbitrary', 'dep:serde', 'dep:serde_derive', 'dep:serde_json']

src/addr2line.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ impl<'a> Addr2lineModules<'a> {
107107
Some(start) => addr
108108
.checked_sub(start)
109109
.context("address is before the beginning of the text section")?,
110-
None => bail!("no code section found in module"),
110+
None => return Ok(None),
111111
}
112112
};
113113

src/bin/wasm-tools/validate.rs

Lines changed: 21 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use addr2line::LookupResult;
22
use anyhow::{anyhow, Context, Result};
3+
use bitflags::Flags;
34
use rayon::prelude::*;
45
use std::fmt::Write;
56
use std::mem;
@@ -171,38 +172,9 @@ impl Opts {
171172
fn parse_features(arg: &str) -> Result<WasmFeatures> {
172173
let mut ret = WasmFeatures::default();
173174

174-
const FEATURES: &[(&str, WasmFeatures)] = &[
175-
("reference-types", WasmFeatures::REFERENCE_TYPES),
176-
("function-references", WasmFeatures::FUNCTION_REFERENCES),
177-
("simd", WasmFeatures::SIMD),
178-
("threads", WasmFeatures::THREADS),
179-
(
180-
"shared-everything-threads",
181-
WasmFeatures::SHARED_EVERYTHING_THREADS,
182-
),
183-
("bulk-memory", WasmFeatures::BULK_MEMORY),
184-
("multi-value", WasmFeatures::MULTI_VALUE),
185-
("tail-call", WasmFeatures::TAIL_CALL),
186-
("component-model", WasmFeatures::COMPONENT_MODEL),
187-
(
188-
"component-model-values",
189-
WasmFeatures::COMPONENT_MODEL_VALUES,
190-
),
191-
("multi-memory", WasmFeatures::MULTI_MEMORY),
192-
("exception-handling", WasmFeatures::EXCEPTIONS),
193-
("memory64", WasmFeatures::MEMORY64),
194-
("extended-const", WasmFeatures::EXTENDED_CONST),
195-
("floats", WasmFeatures::FLOATS),
196-
(
197-
"saturating-float-to-int",
198-
WasmFeatures::SATURATING_FLOAT_TO_INT,
199-
),
200-
("sign-extension", WasmFeatures::SIGN_EXTENSION),
201-
("mutable-global", WasmFeatures::MUTABLE_GLOBAL),
202-
("relaxed-simd", WasmFeatures::RELAXED_SIMD),
203-
("gc", WasmFeatures::GC),
204-
("legacy-exceptions", WasmFeatures::LEGACY_EXCEPTIONS),
205-
];
175+
fn flag_name(flag: &bitflags::Flag<WasmFeatures>) -> String {
176+
flag.name().to_lowercase().replace('_', "-")
177+
}
206178

207179
for part in arg.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {
208180
let (enable, part) = if let Some(part) = part.strip_prefix("-") {
@@ -212,28 +184,27 @@ fn parse_features(arg: &str) -> Result<WasmFeatures> {
212184
};
213185
match part {
214186
"all" => {
215-
for (name, feature) in FEATURES {
216-
// don't count this under "all" for now.
217-
if *name == "deterministic" {
218-
continue;
219-
}
220-
ret.set(*feature, enable);
187+
for flag in WasmFeatures::FLAGS.iter() {
188+
ret.set(*flag.value(), enable);
221189
}
222190
}
223191

224192
name => {
225-
let (_, feature) = FEATURES.iter().find(|(n, _)| *n == name).ok_or_else(|| {
226-
anyhow!(
227-
"unknown feature `{}`\nValid features: {}",
228-
name,
229-
FEATURES
230-
.iter()
231-
.map(|(name, _)| *name)
232-
.collect::<Vec<_>>()
233-
.join(", "),
234-
)
235-
})?;
236-
ret.set(*feature, enable);
193+
let flag = WasmFeatures::FLAGS
194+
.iter()
195+
.find(|f| flag_name(f) == name)
196+
.ok_or_else(|| {
197+
anyhow!(
198+
"unknown feature `{}`\nValid features: {}",
199+
name,
200+
WasmFeatures::FLAGS
201+
.iter()
202+
.map(flag_name)
203+
.collect::<Vec<_>>()
204+
.join(", "),
205+
)
206+
})?;
207+
ret.set(*flag.value(), enable);
237208
}
238209
}
239210
}

tests/cli/validate-features.wat

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
;; FAIL: validate --features=-all %
2+
3+
(module
4+
(import "x" "y" (global v128))
5+
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
error: SIMD support is not enabled (at offset 0xb)

tests/cli/validate-features2.wat

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
;; RUN: validate --features=-all,simd %
2+
3+
(module
4+
(import "x" "y" (global v128))
5+
)
6+
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
;; FAIL: validate --features=unknown %
2+
3+
(module)
4+
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
error: invalid value 'unknown' for '--features <FEATURES>': unknown feature `unknown`
2+
Valid features: mutable-global, saturating-float-to-int, sign-extension, reference-types, multi-value, bulk-memory, simd, relaxed-simd, threads, shared-everything-threads, tail-call, floats, multi-memory, exceptions, memory64, extended-const, component-model, function-references, memory-control, gc, custom-page-sizes, component-model-values, component-model-nested-names, component-model-more-flags, legacy-exceptions
3+
4+
For more information, try '--help'.

0 commit comments

Comments
 (0)