Add support for `--help` flag.
This CL switches from using `getopts` to using `clap`:
- `clap` has built-in support for `--help`
- `clap` in general seems to have a nicer API (e.g. `getopts` required
somewhat repetitive/redundant calls to `reqopt` and `opt_str`, passing
the same `H_OUT` in both cases).
PiperOrigin-RevId: 477774149
diff --git a/cc_bindings_from_rs/BUILD b/cc_bindings_from_rs/BUILD
index ea0702e..ded6e94 100644
--- a/cc_bindings_from_rs/BUILD
+++ b/cc_bindings_from_rs/BUILD
@@ -35,8 +35,8 @@
"//common:rust_allocator_shims",
"//common:token_stream_printer",
"@crate_index//:anyhow",
+ "@crate_index//:clap",
"@crate_index//:either",
- "@crate_index//:getopts",
"@crate_index//:itertools",
"@crate_index//:proc-macro2",
"@crate_index//:quote",
diff --git a/cc_bindings_from_rs/README.md b/cc_bindings_from_rs/README.md
index ee30e5f..52a2174 100644
--- a/cc_bindings_from_rs/README.md
+++ b/cc_bindings_from_rs/README.md
@@ -21,7 +21,7 @@
$ bazel run \
//cc_bindings_from_rs:cc_bindings_from_rs_legacy_toolchain_runner -- \
- --h_out=$(pwd)/test.h -- \
+ --h-out=$(pwd)/test.h -- \
$(pwd)/test.rs \
--crate-type=lib \
--sysroot $(pwd)/third_party/unsupported_toolchains/rust/toolchains/nightly
diff --git a/cc_bindings_from_rs/cc_bindings_from_rs.rs b/cc_bindings_from_rs/cc_bindings_from_rs.rs
index f503d63..23abcdf 100644
--- a/cc_bindings_from_rs/cc_bindings_from_rs.rs
+++ b/cc_bindings_from_rs/cc_bindings_from_rs.rs
@@ -147,7 +147,7 @@
fn run_with_tcx(cmdline: &Cmdline, tcx: TyCtxt) -> anyhow::Result<()> {
let bindings = GeneratedBindings::generate(tcx);
- write_file(cmdline.h_out(), tokens_to_string(bindings.h_body)?.as_str())
+ write_file(&cmdline.h_out, tokens_to_string(bindings.h_body)?.as_str())
}
/// Main entrypoint that (unlike `main`) doesn't do any intitializations that
@@ -155,7 +155,7 @@
/// `install_ice_hook`) and therefore can be used from the tests module below.
fn run_with_cmdline_args(args: &[String]) -> anyhow::Result<()> {
let cmdline = Cmdline::new(args)?;
- bindings_driver::run_after_analysis_and_stop(cmdline.rustc_args(), |tcx| {
+ bindings_driver::run_after_analysis_and_stop(&cmdline.rustc_args, |tcx| {
run_with_tcx(&cmdline, tcx)
})
}
@@ -268,7 +268,7 @@
let mut args = vec![
"cc_bindings_from_rs_unittest_executable".to_string(),
- format!("--h_out={}", h_path.display()),
+ format!("--h-out={}", h_path.display()),
];
args.extend(self.extra_crubit_args.iter().cloned());
args.extend([
@@ -317,7 +317,11 @@
.expect_err("--unrecognized_crubit_flag should trigger an error");
let msg = err.to_string();
- assert_eq!("Unrecognized option: 'unrecognized-crubit-flag'", msg);
+ assert!(
+ msg.contains("Found argument '--unrecognized-crubit-flag' which wasn't expected"),
+ "msg = {}",
+ msg,
+ );
Ok(())
}
@@ -336,12 +340,12 @@
#[test]
fn test_invalid_h_out_path() -> anyhow::Result<()> {
- // Tests not only the specific problem of an invalid `--h_out` argument, but
+ // Tests not only the specific problem of an invalid `--h-out` argument, but
// also tests that errors from `bindings_main::main` are propagated.
let err = TestArgs::default_args()?
.with_h_path("../..")
.run()
- .expect_err("Unwriteable --h_out should trigger an error");
+ .expect_err("Unwriteable --h-out should trigger an error");
let msg = err.to_string();
assert_eq!("Error when writing to ../..", msg);
diff --git a/cc_bindings_from_rs/cmdline.rs b/cc_bindings_from_rs/cmdline.rs
index 8c81174..8a793a5 100644
--- a/cc_bindings_from_rs/cmdline.rs
+++ b/cc_bindings_from_rs/cmdline.rs
@@ -2,37 +2,46 @@
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-use getopts::{Fail, Options};
-use std::path::Path;
+use anyhow::Result;
+use clap::Parser;
+use std::path::PathBuf;
-#[derive(Debug)]
+#[derive(Debug, Parser)]
+#[clap(name = "cc_bindings_from_rs")]
+#[clap(about = "Generates C++ bindings for a Rust crate", long_about = None)]
pub struct Cmdline {
- h_out: String,
- rustc_args: Vec<String>,
+ /// Output path for C++ header file with bindings.
+ #[clap(long, value_parser, value_name = "FILE")]
+ pub h_out: PathBuf,
+
+ /// Command line arguments of the Rust compiler.
+ #[clap(last = true, value_parser)]
+ pub rustc_args: Vec<String>,
}
-const H_OUT: &str = "h_out";
-
impl Cmdline {
- pub fn new(args: &[String]) -> Result<Self, Fail> {
+ pub fn new(args: &[String]) -> Result<Self> {
+ assert_ne!(
+ 0,
+ args.len(),
+ "`args` should include the name of the executable (i.e. argsv[0])"
+ );
+ let exe_name = args[0].clone();
+
// Ensure that `@file` expansion also covers *our* args.
let args = rustc_driver::args::arg_expand_all(args);
- let matches = Options::new()
- .reqopt("", H_OUT, "output path for C++ header file with bindings", "FILE")
- .parse(&args)?;
- let h_out =
- matches.opt_str(H_OUT).expect("getopts should enforce presence of --h_out `reqopt`");
+ // Parse `args` using the parser `derive`d by the `clap` crate.
+ let mut cmdline = Self::try_parse_from(args)?;
- Ok(Self { h_out, rustc_args: matches.free })
- }
+ // For compatibility with `rustc_driver` expectations, we prepend `exe_name` to
+ // `rustc_args. This is needed, because `rustc_driver::RunCompiler::new`
+ // expects that its `at_args` includes the name of the executable -
+ // `handle_options` in `rustc_driver/src/lib.rs` throws away the first
+ // element.
+ cmdline.rustc_args.insert(0, exe_name);
- pub fn h_out(&self) -> &Path {
- Path::new(&self.h_out)
- }
-
- pub fn rustc_args(&self) -> &[String] {
- &self.rustc_args
+ Ok(cmdline)
}
}
@@ -41,9 +50,10 @@
use super::*;
use itertools::Itertools;
+ use std::path::Path;
use tempfile::tempdir;
- fn new_cmdline<'a>(args: impl IntoIterator<Item = &'a str>) -> Result<Cmdline, Fail> {
+ fn new_cmdline<'a>(args: impl IntoIterator<Item = &'a str>) -> Result<Cmdline> {
// When `Cmdline::new` is invoked from `main.rs`, it includes not only the
// "real" cmdline arguments, but also the name of the executable.
let args = std::iter::once("cc_bindings_from_rs_unittest_executable")
@@ -54,47 +64,54 @@
}
#[test]
- fn test_h_out_happy_path() -> Result<(), Fail> {
- let cmdline = new_cmdline(["--h_out=foo.h"])?;
- assert_eq!(Path::new("foo.h"), cmdline.h_out());
- Ok(())
+ fn test_h_out_happy_path() {
+ let cmdline = new_cmdline(["--h-out=foo.h"]).expect("This is a happy path");
+ assert_eq!(Path::new("foo.h"), cmdline.h_out);
}
#[test]
- fn test_h_out_missing() {
- match new_cmdline([]) {
- Err(Fail::OptionMissing(arg)) if arg == H_OUT => (),
- other => panic!("Unexpected success or unrecognized error: {:?}", other),
- }
- }
-
- #[test]
- fn test_h_out_without_arg() {
- match new_cmdline(["--h_out"]) {
- Err(Fail::ArgumentMissing(arg)) if arg == H_OUT => (),
- other => panic!("Unexpected success or unrecognized error: {:?}", other),
- }
- }
-
- #[test]
- fn test_h_out_duplicated() {
- match new_cmdline(["--h_out=foo.h", "--h_out=bar.h"]) {
- Err(Fail::OptionDuplicated(arg)) if arg == H_OUT => (),
- other => panic!("Unexpected success or unrecognized error: {:?}", other),
- }
- }
-
- #[test]
- fn test_rustc_args_happy_path() -> Result<(), Fail> {
+ fn test_rustc_args_happy_path() {
// Note that this test would fail without the `--` separator.
- let cmdline = new_cmdline(["--h_out=foo.h", "--", "test.rs", "--crate-type=lib"])?;
- let rustc_args = cmdline.rustc_args();
+ let cmdline = new_cmdline(["--h-out=foo.h", "--", "test.rs", "--crate-type=lib"])
+ .expect("This is a happy path");
+ let rustc_args = &cmdline.rustc_args;
assert!(
- itertools::equal(&["cc_bindings_from_rs_unittest_executable", "test.rs", "--crate-type=lib"], rustc_args),
+ itertools::equal(
+ ["cc_bindings_from_rs_unittest_executable", "test.rs", "--crate-type=lib"],
+ rustc_args
+ ),
"rustc_args = {:?}",
- rustc_args
+ rustc_args,
);
- Ok(())
+ }
+
+ #[test]
+ fn test_help() {
+ // This test has multiple purposes:
+ // - Direct/obvious purpose: testing that `--help` works
+ // - Double-checking the overall shape of our cmdline "API" (i.e. verification
+ // that the way we use `clap` attributes results in the desired cmdline
+ // "API").
+ // - Exhaustively checking runtime asserts (assumming that tests run in a debug
+ // build; other tests also trigger these asserts). See also:
+ // - https://github.com/clap-rs/clap/issues/2740#issuecomment-907240414
+ // - `clap::builder::App::debug_assert`
+ let anyhow_err = new_cmdline(["--help"]).expect_err("--help should trigger an error");
+ let clap_err = anyhow_err.downcast::<clap::Error>().expect("Expecting `clap` error");
+ let expected_msg = r#"cc_bindings_from_rs
+Generates C++ bindings for a Rust crate
+
+USAGE:
+ cc_bindings_from_rs_unittest_executable --h-out <FILE> [-- <RUSTC_ARGS>...]
+
+ARGS:
+ <RUSTC_ARGS>... Command line arguments of the Rust compiler
+
+OPTIONS:
+ --h-out <FILE> Output path for C++ header file with bindings
+ -h, --help Print help information
+"#;
+ assert_eq!(expected_msg, clap_err.to_string());
}
#[test]
@@ -103,16 +120,19 @@
let tmpfile = tmpdir.path().join("herefile");
std::fs::write(
&tmpfile,
- &["--h_out=foo.h", "--", "test.rs", "--crate-type=lib"].join("\n"),
+ &["--h-out=foo.h", "--", "test.rs", "--crate-type=lib"].join("\n"),
)?;
- let cmdline = Cmdline::new(&[format!("@{}", tmpfile.display())])?;
- assert_eq!(Path::new("foo.h"), cmdline.h_out());
- let rustc_args = cmdline.rustc_args();
+ let flag_file_arg = format!("@{}", tmpfile.display());
+ let cmdline = new_cmdline([flag_file_arg.as_str()]).expect("No errors expected");
+ assert_eq!(Path::new("foo.h"), cmdline.h_out);
+ let rustc_args = &cmdline.rustc_args;
assert!(
- itertools::equal(&["test.rs", "--crate-type=lib"], rustc_args),
+ itertools::equal(
+ ["cc_bindings_from_rs_unittest_executable", "test.rs", "--crate-type=lib"],
+ rustc_args),
"rustc_args = {:?}",
- rustc_args
+ rustc_args,
);
Ok(())
}
