bytecodealliance / bytecodealliance/wasm-tools
wasm-wave: parsing a `flags` value reorders it alphabetically, so a WAVE round trip is not the identity
- Dominant language
- Rust
- Stars
- 1.8k
- Forks
- 351
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 46
Description
**Crate:** `wasm-wave` 0.254.0
### What happens
`to_string` renders a `flags` value in the order the `Val` holds it. `from_str`
returns the labels sorted alphabetically. So `from_str(ty, &to_string(v))? != v`
for any set of two or more flags whose order is not already alphabetical.
```
input Flags(["write", "exec"])
rendered {write, exec}
parsed Flags(["exec", "write"])
equal? false
```
### Reproducer
```rust
// wasm-wave = "0.254", wasmtime = { version = "48", features = ["wave"] }
use wasmtime::component::{Component, Val};
use wasmtime::{Config, Engine};
const WAT: &str = r#"
(component
(type $perm (flags "read" "write" "exec"))
(export "perm" (type $perm))
)
"#;
fn main() {
let mut config = Config::new();
config.wasm_component_model(true);
let engine = Engine::new(&config).unwrap();
let component = Component::new(&engine, WAT).unwrap();
let ty = component
.component_type()
.exports(&engine)
.find_map(|(name, item)| match item.ty {
wasmtime::component::types::ComponentItem::Type(ty) if name == "perm" => Some(ty),
_ => None,
})
.unwrap();
let value = Val::Flags(vec!["write".into(), "exec".into()]);
let text = wasm_wave::to_string(&value).unwrap();
let parsed: Val = wasm_wave::from_str(&ty, &text).unwrap();
println!("{value:?} -> {text} -> {parsed:?} equal? {}", parsed == value);
}
```
### Cause
`Parser::finish_flags` (`src/parser.rs`) collects into a `BTreeMap` in order to
reject duplicate labels, then returns `flags.into_values()`. The map is keyed by
label, so iteration order is alphabetical rather than source order.
The adjacent `finish_record` solves the same problem without reordering: a
`BTreeSet` named `seen` does the duplicate check while a `Vec` keeps the
children in source order. Applying that shape to `finish_flags` looks like it
would be a small change.
### Why it matters
For the component model a flag set is a bitfield and order carries no meaning,
so this may well be deliberate canonicalisation. The problem is that
`wasmtime::component::Val` derives `PartialEq`, which compares
`Flags(Vec)` positionally. Any code that round-trips a value through
WAVE and compares it with `==` — property tests and golden-file tests
especially — sees a spurious inequality, and the failure points at the value
rather than at the ordering.
Two things would each resolve it, and either is fine from the outside:
- preserve source order in `finish_flags`, matching `to_string` and
`finish_record`; or
- keep the canonicalisation and document on `from_str`/`WasmValue` that flag
order is not preserved, so callers know to compare flags as sets.
I have no view on which is right — reporting it because the asymmetry between
render and parse is currently silent, and because `finish_record` next door
suggests the ordering may not be intentional.
### Context
Found while property-testing a WAVE round trip in a component host
(`from_wave(ty, &to_wave(v))? == v` as the oracle). Worked around locally by
comparing flag sets order-insensitively.
Contributor guide
Research direction
Start in wasm-wave/src/parser.rs at Parser::finish_flags and compare it with the adjacent finish_record implementation. Use the provided WAVE round-trip reproducer as the entry point, then locate the existing parser or property tests. Done means the chosen ordering or documentation behavior is explicit and the round-trip expectation is covered by a regression test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, wasm
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 54/100