symcheck: Make target a positional argument

This makes it more obvious what we intend to check rather than looking
for `--target`.

Author: tgross35

ci/run.sh

@@ -54,29 +54,26 @@ symcheck=(cargo run -p symbol-check --release)
 [[ "$target" = "wasm"* ]] && symcheck+=(--features wasm)
 symcheck+=(-- build-and-check)
 
-"${symcheck[@]}" -p compiler_builtins --target "$target"
-"${symcheck[@]}" -p compiler_builtins --target "$target" --release
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features c
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features c --release
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm --release
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128
-"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 --release
+"${symcheck[@]}" "$target" -- -p compiler_builtins
+"${symcheck[@]}" "$target" -- -p compiler_builtins --release
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features c
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features c --release
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features no-asm
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features no-asm --release
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features no-f16-f128
+"${symcheck[@]}" "$target" -- -p compiler_builtins --features no-f16-f128 --release
 
 run_intrinsics_test() {
-    args=(
-        --target "$target" --verbose \
-        --manifest-path builtins-test-intrinsics/Cargo.toml
-    )
-    args+=( "$@" )
+    cargo_args=(--verbose --manifest-path builtins-test-intrinsics/Cargo.toml)
+    cargo_args+=("$@")
 
     # symcheck also checks the results of builtins-test-intrinsics
-    "${symcheck[@]}" "${args[@]}"
+    "${symcheck[@]}" "$target" -- "${cargo_args[@]}"
 
     # FIXME: we get access violations on Windows, our entrypoint may need to
     # be tweaked.
     if [ "${BUILD_ONLY:-}" != "1" ] && ! [[ "$target" = *"windows"* ]]; then
-        cargo run "${args[@]}"
+        cargo run --target "$target" "${cargo_args[@]}"
     fi
 }
 

crates/symbol-check/src/main.rs

@@ -18,10 +18,12 @@ const CHECK_EXTENSIONS: &[Option<&str>] = &[Some("rlib"), Some("a"), Some("exe")
 
 const USAGE: &str = "Usage:
 
-    symbol-check build-and-check CARGO_ARGS ...
+    symbol-check build-and-check [TARGET] -- CARGO_ARGS ...
 
-Cargo will get invoked with `CARGO_ARGS` and all output
+Cargo will get invoked with `CARGO_ARGS` and the specified target. All output
 `compiler_builtins*.rlib` files will be checked.
+
+If TARGET is not specified, the host target is used.
 ";
 
 fn main() {
@@ -30,11 +32,13 @@ fn main() {
     let args_ref = args.iter().map(String::as_str).collect::<Vec<_>>();
 
     match &args_ref[1..] {
-        ["build-and-check", "--target", target, args @ ..] if !args.is_empty() => {
-            run_build_and_check(Some(target), args);
+        ["build-and-check", target, "--", args @ ..] if !args.is_empty() => {
+            check_cargo_args(args);
+            run_build_and_check(target, args);
         }
-        ["build-and-check", args @ ..] if !args.is_empty() => {
-            run_build_and_check(None, args);
+        ["build-and-check", "--", args @ ..] if !args.is_empty() => {
+            check_cargo_args(args);
+            run_build_and_check(&host_target(), args);
         }
         _ => {
             println!("{USAGE}");
@@ -43,14 +47,14 @@ fn main() {
     }
 }
 
-fn run_build_and_check(target: Option<&str>, args: &[&str]) {
-    let paths = exec_cargo_with_args(target, args);
-    for path in paths {
-        println!("Checking {}", path.display());
-        let archive = Archive::from_path(&path);
-
-        verify_no_duplicates(&archive);
-        verify_core_symbols(&archive);
+/// Make sure `--target` isn't passed to avoid confusion (since it should be proivded only once,
+/// positionally).
+fn check_cargo_args(args: &[&str]) {
+    for arg in args {
+        assert!(
+            !arg.contains("--target"),
+            "target must be passed positionally. {USAGE}"
+        );
     }
 }
 
@@ -68,15 +72,20 @@ fn host_target() -> String {
         .to_owned()
 }
 
+fn run_build_and_check(target: &str, args: &[&str]) {
+    let paths = exec_cargo_with_args(target, args);
+    for path in paths {
+        println!("Checking {}", path.display());
+        let archive = Archive::from_path(&path);
+
+        verify_no_duplicates(&archive);
+        verify_core_symbols(&archive);
+    }
+}
+
 /// Run `cargo build` with the provided additional arguments, collecting the list of created
 /// libraries.
-fn exec_cargo_with_args(target: Option<&str>, args: &[&str]) -> Vec<PathBuf> {
-    let mut host = String::new();
-    let target = target.unwrap_or_else(|| {
-        host = host_target();
-        host.as_str()
-    });
-
+fn exec_cargo_with_args(target: &str, args: &[&str]) -> Vec<PathBuf> {
     let mut cmd = Command::new("cargo");
     cmd.args(["build", "--target", target, "--message-format=json"])
         .args(args)