Skip to content

Commit 654cb0d

Browse files
borsbors
committed
Auto merge of #3621 - RalfJung:argparse, r=RalfJung
use a little arg-parsing helper for miri-script
2 parents f763474 + 060fd17 commit 654cb0d

File tree

5 files changed

+262
-147
lines changed

5 files changed

+262
-147
lines changed

‎miri-script/src/args.rs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
use std::env;
2+
use std::iter;
3+
4+
use anyhow::{bail, Result};
5+
6+
pub struct Args {
7+
args: iter::Peekable<env::Args>,
8+
/// Set to `true` once we saw a `--`.
9+
terminated: bool,
10+
}
11+
12+
impl Args {
13+
pub fn new() -> Self {
14+
let mut args = Args { args: env::args().peekable(), terminated: false };
15+
args.args.next().unwrap(); // skip program name
16+
args
17+
}
18+
19+
/// Get the next argument without any interpretation.
20+
pub fn next_raw(&mut self) -> Option<String> {
21+
self.args.next()
22+
}
23+
24+
/// Consume a `-$f` flag if present.
25+
pub fn get_short_flag(&mut self, flag: char) -> Result<bool> {
26+
if self.terminated {
27+
return Ok(false);
28+
}
29+
if let Some(next) = self.args.peek() {
30+
if let Some(next) = next.strip_prefix("-") {
31+
if let Some(next) = next.strip_prefix(flag) {
32+
if next.is_empty() {
33+
self.args.next().unwrap(); // consume this argument
34+
return Ok(true);
35+
} else {
36+
bail!("`-{flag}` followed by value");
37+
}
38+
}
39+
}
40+
}
41+
Ok(false)
42+
}
43+
44+
/// Consume a `--$name` flag if present.
45+
pub fn get_long_flag(&mut self, name: &str) -> Result<bool> {
46+
if self.terminated {
47+
return Ok(false);
48+
}
49+
if let Some(next) = self.args.peek() {
50+
if let Some(next) = next.strip_prefix("--") {
51+
if next == name {
52+
self.args.next().unwrap(); // consume this argument
53+
return Ok(true);
54+
}
55+
}
56+
}
57+
Ok(false)
58+
}
59+
60+
/// Consume a `--$name val` or `--$name=val` option if present.
61+
pub fn get_long_opt(&mut self, name: &str) -> Result<Option<String>> {
62+
assert!(!name.is_empty());
63+
if self.terminated {
64+
return Ok(None);
65+
}
66+
let Some(next) = self.args.peek() else { return Ok(None) };
67+
let Some(next) = next.strip_prefix("--") else { return Ok(None) };
68+
let Some(next) = next.strip_prefix(name) else { return Ok(None) };
69+
// Starts with `--flag`.
70+
Ok(if let Some(val) = next.strip_prefix("=") {
71+
// `--flag=val` form
72+
let val = val.into();
73+
self.args.next().unwrap(); // consume this argument
74+
Some(val)
75+
} else if next.is_empty() {
76+
// `--flag val` form
77+
self.args.next().unwrap(); // consume this argument
78+
let Some(val) = self.args.next() else { bail!("`--{name}` not followed by value") };
79+
Some(val)
80+
} else {
81+
// Some unrelated flag, like `--flag-more` or so.
82+
None
83+
})
84+
}
85+
86+
/// Consume a `--$name=val` or `--$name` option if present; the latter
87+
/// produces a default value. (`--$name val` is *not* accepted for this form
88+
/// of argument, it understands `val` already as the next argument!)
89+
pub fn get_long_opt_with_default(
90+
&mut self,
91+
name: &str,
92+
default: &str,
93+
) -> Result<Option<String>> {
94+
assert!(!name.is_empty());
95+
if self.terminated {
96+
return Ok(None);
97+
}
98+
let Some(next) = self.args.peek() else { return Ok(None) };
99+
let Some(next) = next.strip_prefix("--") else { return Ok(None) };
100+
let Some(next) = next.strip_prefix(name) else { return Ok(None) };
101+
// Starts with `--flag`.
102+
Ok(if let Some(val) = next.strip_prefix("=") {
103+
// `--flag=val` form
104+
let val = val.into();
105+
self.args.next().unwrap(); // consume this argument
106+
Some(val)
107+
} else if next.is_empty() {
108+
// `--flag` form
109+
self.args.next().unwrap(); // consume this argument
110+
Some(default.into())
111+
} else {
112+
// Some unrelated flag, like `--flag-more` or so.
113+
None
114+
})
115+
}
116+
117+
/// Returns the next free argument or uninterpreted flag, or `None` if there are no more
118+
/// arguments left. `--` is returned as well, but it is interpreted in the sense that no more
119+
/// flags will be parsed after this.
120+
pub fn get_other(&mut self) -> Option<String> {
121+
if self.terminated {
122+
return self.args.next();
123+
}
124+
let next = self.args.next()?;
125+
if next == "--" {
126+
self.terminated = true; // don't parse any more flags
127+
// This is where our parser is special, we do yield the `--`.
128+
}
129+
Some(next)
130+
}
131+
132+
/// Return the rest of the aguments entirely unparsed.
133+
pub fn remainder(self) -> Vec<String> {
134+
self.args.collect()
135+
}
136+
}

‎miri-script/src/commands.rs

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ impl MiriEnv {
2525
/// Returns the location of the sysroot.
2626
///
2727
/// If the target is None the sysroot will be built for the host machine.
28-
fn build_miri_sysroot(&mut self, quiet: bool, target: Option<&OsStr>) -> Result<PathBuf> {
28+
fn build_miri_sysroot(
29+
&mut self,
30+
quiet: bool,
31+
target: Option<impl AsRef<OsStr>>,
32+
) -> Result<PathBuf> {
2933
if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") {
3034
// Sysroot already set, use that.
3135
return Ok(miri_sysroot.into());
@@ -37,14 +41,17 @@ impl MiriEnv {
3741
self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?;
3842
self.build(&manifest_path, &[], quiet)?;
3943

40-
let target_flag =
41-
if let Some(target) = target { vec![OsStr::new("--target"), target] } else { vec![] };
44+
let target_flag = if let Some(target) = &target {
45+
vec![OsStr::new("--target"), target.as_ref()]
46+
} else {
47+
vec![]
48+
};
4249
let target_flag = &target_flag;
4350

4451
if !quiet {
4552
eprint!("$ cargo miri setup");
46-
if let Some(target) = target {
47-
eprint!(" --target {target}", target = target.to_string_lossy());
53+
if let Some(target) = &target {
54+
eprint!(" --target {target}", target = target.as_ref().to_string_lossy());
4855
}
4956
eprintln!();
5057
}
@@ -157,8 +164,8 @@ impl Command {
157164
Command::Build { flags } => Self::build(flags),
158165
Command::Check { flags } => Self::check(flags),
159166
Command::Test { bless, flags, target } => Self::test(bless, flags, target),
160-
Command::Run { dep, verbose, many_seeds, flags } =>
161-
Self::run(dep, verbose, many_seeds, flags),
167+
Command::Run { dep, verbose, many_seeds, target, edition, flags } =>
168+
Self::run(dep, verbose, many_seeds, target, edition, flags),
162169
Command::Fmt { flags } => Self::fmt(flags),
163170
Command::Clippy { flags } => Self::clippy(flags),
164171
Command::Cargo { flags } => Self::cargo(flags),
@@ -169,7 +176,7 @@ impl Command {
169176
}
170177
}
171178

172-
fn toolchain(flags: Vec<OsString>) -> Result<()> {
179+
fn toolchain(flags: Vec<String>) -> Result<()> {
173180
// Make sure rustup-toolchain-install-master is installed.
174181
which::which("rustup-toolchain-install-master")
175182
.context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?;
@@ -364,7 +371,7 @@ impl Command {
364371
Ok(())
365372
}
366373

367-
fn bench(target: Option<OsString>, benches: Vec<OsString>) -> Result<()> {
374+
fn bench(target: Option<String>, benches: Vec<String>) -> Result<()> {
368375
// The hyperfine to use
369376
let hyperfine = env::var("HYPERFINE");
370377
let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none");
@@ -378,14 +385,14 @@ impl Command {
378385
let sh = Shell::new()?;
379386
sh.change_dir(miri_dir()?);
380387
let benches_dir = "bench-cargo-miri";
381-
let benches = if benches.is_empty() {
388+
let benches: Vec<OsString> = if benches.is_empty() {
382389
sh.read_dir(benches_dir)?
383390
.into_iter()
384391
.filter(|path| path.is_dir())
385392
.map(Into::into)
386393
.collect()
387394
} else {
388-
benches.to_owned()
395+
benches.into_iter().map(Into::into).collect()
389396
};
390397
let target_flag = if let Some(target) = target {
391398
let mut flag = OsString::from("--target=");
@@ -409,36 +416,36 @@ impl Command {
409416
Ok(())
410417
}
411418

412-
fn install(flags: Vec<OsString>) -> Result<()> {
419+
fn install(flags: Vec<String>) -> Result<()> {
413420
let e = MiriEnv::new()?;
414421
e.install_to_sysroot(e.miri_dir.clone(), &flags)?;
415422
e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?;
416423
Ok(())
417424
}
418425

419-
fn build(flags: Vec<OsString>) -> Result<()> {
426+
fn build(flags: Vec<String>) -> Result<()> {
420427
let e = MiriEnv::new()?;
421428
e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?;
422429
e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?;
423430
Ok(())
424431
}
425432

426-
fn check(flags: Vec<OsString>) -> Result<()> {
433+
fn check(flags: Vec<String>) -> Result<()> {
427434
let e = MiriEnv::new()?;
428435
e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?;
429436
e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?;
430437
Ok(())
431438
}
432439

433-
fn clippy(flags: Vec<OsString>) -> Result<()> {
440+
fn clippy(flags: Vec<String>) -> Result<()> {
434441
let e = MiriEnv::new()?;
435442
e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?;
436443
e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?;
437444
e.clippy(path!(e.miri_dir / "miri-script" / "Cargo.toml"), &flags)?;
438445
Ok(())
439446
}
440447

441-
fn cargo(flags: Vec<OsString>) -> Result<()> {
448+
fn cargo(flags: Vec<String>) -> Result<()> {
442449
let e = MiriEnv::new()?;
443450
let toolchain = &e.toolchain;
444451
// We carefully kept the working dir intact, so this will run cargo *on the workspace in the
@@ -447,7 +454,7 @@ impl Command {
447454
Ok(())
448455
}
449456

450-
fn test(bless: bool, mut flags: Vec<OsString>, target: Option<OsString>) -> Result<()> {
457+
fn test(bless: bool, mut flags: Vec<String>, target: Option<String>) -> Result<()> {
451458
let mut e = MiriEnv::new()?;
452459

453460
// Prepare a sysroot.
@@ -475,21 +482,30 @@ impl Command {
475482
dep: bool,
476483
verbose: bool,
477484
many_seeds: Option<Range<u32>>,
478-
mut flags: Vec<OsString>,
485+
target: Option<String>,
486+
edition: Option<String>,
487+
flags: Vec<String>,
479488
) -> Result<()> {
480489
let mut e = MiriEnv::new()?;
481-
let target = arg_flag_value(&flags, "--target");
482-
483-
// Scan for "--edition", set one ourselves if that flag is not present.
484-
let have_edition = arg_flag_value(&flags, "--edition").is_some();
485-
if !have_edition {
486-
flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.`
490+
// More flags that we will pass before `flags`
491+
// (because `flags` may contain `--`).
492+
let mut early_flags = Vec::<OsString>::new();
493+
494+
// Add target, edition to flags.
495+
if let Some(target) = &target {
496+
early_flags.push("--target".into());
497+
early_flags.push(target.into());
498+
}
499+
if verbose {
500+
early_flags.push("--verbose".into());
487501
}
502+
early_flags.push("--edition".into());
503+
early_flags.push(edition.as_deref().unwrap_or("2021").into());
488504

489-
// Prepare a sysroot, and add it to the flags.
505+
// Prepare a sysroot, add it to the flags.
490506
let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?;
491-
flags.push("--sysroot".into());
492-
flags.push(miri_sysroot.into());
507+
early_flags.push("--sysroot".into());
508+
early_flags.push(miri_sysroot.into());
493509

494510
// Compute everything needed to run the actual command. Also add MIRIFLAGS.
495511
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
@@ -515,7 +531,7 @@ impl Command {
515531
};
516532
cmd.set_quiet(!verbose);
517533
// Add Miri flags
518-
let cmd = cmd.args(&miri_flags).args(seed_flag).args(&flags);
534+
let cmd = cmd.args(&miri_flags).args(&seed_flag).args(&early_flags).args(&flags);
519535
// And run the thing.
520536
Ok(cmd.run()?)
521537
};
@@ -534,7 +550,7 @@ impl Command {
534550
Ok(())
535551
}
536552

537-
fn fmt(flags: Vec<OsString>) -> Result<()> {
553+
fn fmt(flags: Vec<String>) -> Result<()> {
538554
use itertools::Itertools;
539555

540556
let e = MiriEnv::new()?;
@@ -556,6 +572,6 @@ impl Command {
556572
.filter_ok(|item| item.file_type().is_file())
557573
.map_ok(|item| item.into_path());
558574

559-
e.format_files(files, &e.toolchain[..], &config_path, &flags[..])
575+
e.format_files(files, &e.toolchain[..], &config_path, &flags)
560576
}
561577
}

0 commit comments

Comments
 (0)