Skip to content

Commit 91dcfa8

Browse files
alexcrichtonalexcrichton
authored
Fix wasm-ld flags which require = (#23)
I misunderstood flag parsing in `wasm-ld` and it looks like some flags require `=` for their values where for some it's optional and can be specified with a space instead. This implements distinguishing these arguments and then additionally rereads the `-help` page of `wasm-ld` and copies everything over. Closes #22
1 parent e060108 commit 91dcfa8

2 files changed

Lines changed: 104 additions & 23 deletions

File tree

‎src/main.rs‎

Lines changed: 88 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,20 @@ use std::process::Command;
88
use std::str::FromStr;
99
use wasmparser::Payload;
1010

11+
/// Representation of a flag passed to `wasm-ld`
12+
///
13+
/// Note that the parsing of flags in `wasm-ld` is not as uniform as parsing
14+
/// arguments via `clap`. For example if `--foo bar` is supported that doesn't
15+
/// mean that `--foo=bar` is supported. Similarly some options such as `--foo`
16+
/// support optional values as `--foo=bar` but can't be specified as
17+
/// `--foo bar`.
18+
///
19+
/// Finally there's currently only one "weird" flag which is `-shared` which has
20+
/// a single dash but a long name. That's specially handled elsewhere.
21+
///
22+
/// The general goal here is that we want to inherit `wasm-ld`'s CLI but also
23+
/// want to be able to reserve CLI flags for this linker itself, so `wasm-ld`'s
24+
/// arguments are parsed where our own are intermixed.
1125
struct LldFlag {
1226
clap_name: &'static str,
1327
long: Option<&'static str>,
@@ -16,12 +30,37 @@ struct LldFlag {
1630
}
1731

1832
enum FlagValue {
33+
/// This option has no value, e.g. `-f` or `--foo`
1934
None,
20-
Required(&'static str),
35+
36+
/// This option's value must be specified with `=`, for example `--foo=bar`
37+
RequiredEqual(&'static str),
38+
39+
/// This option's value must be specified with ` `, for example `--foo bar`.
40+
///
41+
/// I think that `wasm-ld` supports both `-f foo` and `-ffoo` for
42+
/// single-character flags, but I haven't tested as putting a space seems to
43+
/// work.
44+
RequiredSpace(&'static str),
45+
46+
/// This option's value is optional but if specified it must use an `=` for
47+
/// example `--foo=bar` or `--foo`.
2148
Optional(&'static str),
2249
}
2350

51+
/// This is a large macro which is intended to take CLI-looking syntax and turn
52+
/// each individual flag into a `LldFlag` specified above.
2453
macro_rules! flag {
54+
// Long options specified as:
55+
//
56+
// -f / --foo
57+
//
58+
// or just
59+
//
60+
// --foo
61+
//
62+
// Options can look like `--foo`, `--foo=bar`, `--foo[=bar]`, or
63+
// `--foo bar` to match the kinds of flags that LLD supports.
2564
($(-$short:ident /)? --$($flag:tt)*) => {
2665
LldFlag {
2766
clap_name: concat!("long_", $(stringify!($flag),)*),
@@ -31,30 +70,50 @@ macro_rules! flag {
3170
}
3271
};
3372

34-
(-$flag:tt $(=$val:tt)?) => {
73+
// Short options specified as `-f` or `-f foo`.
74+
(-$flag:tt $($val:tt)*) => {
3575
LldFlag {
3676
clap_name: concat!("short_", stringify!($flag)),
3777
long: None,
3878
short: Some(flag!(@char $flag)),
39-
value: flag!(@value $(=$val)?),
79+
value: flag!(@value $flag $($val)*),
4080
}
4181
};
4282

43-
(@name [$($name:tt)*] $n:ident $($rest:tt)*) => (flag!(@name [$($name)* $n] $($rest)*));
44-
(@name [$($name:tt)*] - $($rest:tt)*) => (flag!(@name [$($name)* -] $($rest)*));
45-
(@name [$($name:tt)*] = $($rest:tt)*) => (concat!($(stringify!($name),)*));
46-
(@name [$($name:tt)*] [= $($rest:tt)*]) => (concat!($(stringify!($name),)*));
83+
// Generates the long name of a flag, collected within the `[]` argument to
84+
// this macro. This will iterate over the flag given as the rest of the
85+
// macro arguments and collect values into `[...]` and recurse.
86+
//
87+
// The first recursion case handles `foo-bar-baz=..` where Rust tokenizes
88+
// this as `foo` then `-` then `bar` then ... If this is found then `foo-`
89+
// is added to the name and then the macro recurses.
90+
(@name [$($name:tt)*] $n:ident-$($rest:tt)*) => (flag!(@name [$($name)* $n-] $($rest)*));
91+
// These are the ways options are represented, either `--foo bar`,
92+
// `--foo=bar`, `--foo=bar`, or `--foo`. In all these cases discard the
93+
// value itself and then recurse.
94+
(@name [$($name:tt)*] $n:ident $_value:ident) => (flag!(@name [$($name)* $n]));
95+
(@name [$($name:tt)*] $n:ident=$_value:ident) => (flag!(@name [$($name)* $n]));
96+
(@name [$($name:tt)*] $n:ident[=$_value:ident]) => (flag!(@name [$($name)* $n]));
97+
(@name [$($name:tt)*] $n:ident) => (flag!(@name [$($name)* $n]));
98+
// If there's nothing left then the `$name` has collected everything so
99+
// it's stringifyied and caoncatenated.
47100
(@name [$($name:tt)*]) => (concat!($(stringify!($name),)*));
48101

49-
(@value $n:ident $($rest:tt)*) => (flag!(@value $($rest)*));
50-
(@value - $($rest:tt)*) => (flag!(@value $($rest)*));
51-
(@value = $name:ident) => (FlagValue::Required(stringify!($name)));
52-
(@value [= $name:ident]) => (FlagValue::Optional(stringify!($name)));
53-
(@value) => (FlagValue::None);
54-
102+
// This parses the value-style of the flag given. The recursion here looks
103+
// similar to `@name` above. except that the four terminal cases all
104+
// correspond to different variants of `FlagValue`.
105+
(@value $n:ident - $($rest:tt)*) => (flag!(@value $($rest)*));
106+
(@value $_flag:ident = $name:ident) => (FlagValue::RequiredEqual(stringify!($name)));
107+
(@value $_flag:ident $name:ident) => (FlagValue::RequiredSpace(stringify!($name)));
108+
(@value $_flag:ident [= $name:ident]) => (FlagValue::Optional(stringify!($name)));
109+
(@value $_flag:ident) => (FlagValue::None);
110+
111+
// Helper for flags that have both a long and a short form to parse whether
112+
// a short form was provided.
55113
(@short) => (None);
56114
(@short $name:ident) => (Some(flag!(@char $name)));
57115

116+
// Helper for getting the `char` of a short flag.
58117
(@char $name:ident) => ({
59118
let name = stringify!($name);
60119
assert!(name.len() == 1);
@@ -78,7 +137,7 @@ const LLD_FLAGS: &[LldFlag] = &[
78137
flag! { --dy },
79138
flag! { --emit-relocs },
80139
flag! { --end-lib },
81-
flag! { --entry=SYM },
140+
flag! { --entry SYM },
82141
flag! { --error-limit=N },
83142
flag! { --error-unresolved-symbols },
84143
flag! { --experimental-pic },
@@ -104,13 +163,13 @@ const LLD_FLAGS: &[LldFlag] = &[
104163
flag! { --lto-debug-pass-manager },
105164
flag! { --lto-O=LEVEL },
106165
flag! { --lto-partitions=NUM },
107-
flag! { -L=PATH },
108-
flag! { -l=LIB },
166+
flag! { -L PATH },
167+
flag! { -l LIB },
109168
flag! { --Map=FILE },
110169
flag! { --max-memory=SIZE },
111170
flag! { --merge-data-segments },
112171
flag! { --mllvm=FLAG },
113-
flag! { -m=ARCH },
172+
flag! { -m ARCH },
114173
flag! { --no-check-features },
115174
flag! { --no-color-diagnostics },
116175
flag! { --no-demangle },
@@ -122,7 +181,7 @@ const LLD_FLAGS: &[LldFlag] = &[
122181
flag! { --no-print-gc-sections },
123182
flag! { --no-whole-archive },
124183
flag! { --non_shared },
125-
flag! { -O=LEVEL },
184+
flag! { -O LEVEL },
126185
flag! { --pie },
127186
flag! { --print-gc-sections },
128187
flag! { -M / --print-map },
@@ -149,7 +208,7 @@ const LLD_FLAGS: &[LldFlag] = &[
149208
flag! { --whole-archive },
150209
flag! { --why-extract=MEMBER },
151210
flag! { --wrap=VALUE },
152-
flag! { -z=OPT },
211+
flag! { -z OPT },
153212
];
154213

155214
const LLD_LONG_FLAGS_NONSTANDARD: &[&str] = &["-shared"];
@@ -334,13 +393,17 @@ impl App {
334393
lld_args.push(arg);
335394
}
336395

337-
// LLD doesn't support options of the form `-l=c` or` -O=2` so
338-
// pass these unconditionally as two arguments.
339-
FlagValue::Required(_) => {
396+
FlagValue::RequiredSpace(_) => {
340397
lld_args.push(arg);
341398
lld_args.push(parser.value()?);
342399
}
343400

401+
FlagValue::RequiredEqual(_) => {
402+
arg.push("=");
403+
arg.push(&parser.value()?);
404+
lld_args.push(arg);
405+
}
406+
344407
// If the value is optional then the argument must have an `=`
345408
// in the argument itself.
346409
FlagValue::Optional(_) => {
@@ -568,7 +631,9 @@ fn add_wasm_ld_options(mut command: clap::Command) -> clap::Command {
568631
arg = arg.long(long);
569632
}
570633
arg = match flag.value {
571-
FlagValue::Required(name) => arg.action(ArgAction::Set).value_name(name),
634+
FlagValue::RequiredEqual(name) | FlagValue::RequiredSpace(name) => {
635+
arg.action(ArgAction::Set).value_name(name)
636+
}
572637
FlagValue::Optional(name) => arg
573638
.action(ArgAction::Set)
574639
.value_name(name)

‎tests/all.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,19 @@ fn main() {
101101
);
102102
assert_component(&output);
103103
}
104+
105+
#[test]
106+
fn linker_flags() {
107+
let output = compile(
108+
&[
109+
"-Clink-arg=--max-memory=65536",
110+
"-Clink-arg=-zstack-size=32",
111+
"-Clink-arg=--global-base=2048",
112+
],
113+
r#"
114+
fn main() {
115+
}
116+
"#,
117+
);
118+
assert_component(&output);
119+
}

0 commit comments

Comments
 (0)