Merge pull request #2203 from fitzgen/clean-up-clif-util

Clean up `clif-util`: use anyhow and structopt

.github/workflows/main.yml

@@ -129,7 +129,6 @@ jobs:
     - run: rustup install 1.43.0
     - run: cargo +1.43.0 check -p cranelift-codegen
     - run: cargo +1.43.0 check -p cranelift-wasm
-    - run: cargo +1.43.0 check -p cranelift-tools
 
 
   fuzz_targets:

Cargo.lock

@@ -50,9 +50,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.31"
+version = "1.0.32"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "85bb70cc08ec97ca5450e6eba421deeea5f172c0fc61f78b5357b2a8e8be195f"
+checksum = "6b602bfe940d21c130f3895acd65221e8a61270debe89d628b9cb4e3ccb8569b"
 
 [[package]]
 name = "arbitrary"
@@ -419,6 +419,7 @@ dependencies = [
 name = "cranelift-filetests"
 version = "0.66.0"
 dependencies = [
+ "anyhow",
  "byteorder",
  "cranelift-codegen",
  "cranelift-frontend",
@@ -544,9 +545,9 @@ dependencies = [
 name = "cranelift-tools"
 version = "0.66.0"
 dependencies = [
+ "anyhow",
  "capstone",
  "cfg-if",
- "clap",
  "cranelift",
  "cranelift-codegen",
  "cranelift-entity",
@@ -568,6 +569,7 @@ dependencies = [
  "peepmatic-souper",
  "pretty_env_logger",
  "rayon",
+ "structopt",
  "target-lexicon",
  "term",
  "thiserror",
@@ -1970,9 +1972,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
 
 [[package]]
 name = "structopt"
-version = "0.3.15"
+version = "0.3.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de2f5e239ee807089b62adce73e48c625e0ed80df02c7ab3f068f5db5281065c"
+checksum = "6cc388d94ffabf39b5ed5fadddc40147cb21e605f53db6f8f36a625d27489ac5"
 dependencies = [
  "clap",
  "lazy_static",
@@ -1981,9 +1983,9 @@ dependencies = [
 
 [[package]]
 name = "structopt-derive"
-version = "0.4.8"
+version = "0.4.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "510413f9de616762a4fbeab62509bf15c729603b72d7cd71280fbca431b1c118"
+checksum = "5e2513111825077552a6751dfad9e11ce0fba07d7276a3943a037d7e93e64c5f"
 dependencies = [
  "heck",
  "proc-macro-error",

cranelift/Cargo.toml

@@ -30,12 +30,11 @@ cranelift-simplejit = { path = "simplejit", version = "0.66.0" }
 cranelift-preopt = { path = "preopt", version = "0.66.0" }
 cranelift = { path = "umbrella", version = "0.66.0" }
 filecheck = "0.5.0"
-clap = "2.32.0"
 log = "0.4.8"
 term = "0.6.1"
 capstone = { version = "0.6.0", optional = true }
 wat = { version = "1.0.18", optional = true }
-target-lexicon = "0.10"
+target-lexicon = { version = "0.10", features = ["std"] }
 peepmatic-souper = { path = "./peepmatic/crates/souper", version = "0.66.0", optional = true }
 pretty_env_logger = "0.4.0"
 rayon = { version = "1", optional = true }
@@ -43,6 +42,8 @@ file-per-thread-logger = "0.1.2"
 indicatif = "0.13.0"
 thiserror = "1.0.15"
 walkdir = "2.2"
+anyhow = "1.0.32"
+structopt = "0.3.17"
 
 [features]
 default = ["disas", "wasm", "cranelift-codegen/all-arch", "peepmatic-souper", "souper-harvest"]

cranelift/filetests/Cargo.toml

@@ -25,6 +25,7 @@ memmap = "0.7.0"
 num_cpus = "1.8.0"
 target-lexicon = "0.10"
 thiserror = "1.0.15"
+anyhow = "1.0.32"
 
 [features]
 enable-peepmatic = []

cranelift/filetests/src/concurrent.rs

@@ -4,7 +4,6 @@
 //! concurrently.
 
 use crate::runone;
-use crate::TestResult;
 use cranelift_codegen::dbg::LOG_FILENAME_PREFIX;
 use cranelift_codegen::timing;
 use file_per_thread_logger;
@@ -22,8 +21,14 @@ struct Request(usize, PathBuf);
 
 /// Reply from worker thread,
 pub enum Reply {
-    Starting { jobid: usize, thread_num: usize },
+    Starting {
-    Done { jobid: usize, result: TestResult },
+        jobid: usize,
+        thread_num: usize,
+    },
+    Done {
+        jobid: usize,
+        result: anyhow::Result<Duration>,
+    },
     Tick,
 }
 
@@ -147,11 +152,11 @@ fn worker_thread(
                         // The test panicked, leaving us a `Box<Any>`.
                         // Panics are usually strings.
                         if let Some(msg) = e.downcast_ref::<String>() {
-                            Err(format!("panicked in worker #{}: {}", thread_num, msg))
+                            anyhow::bail!("panicked in worker #{}: {}", thread_num, msg)
                         } else if let Some(msg) = e.downcast_ref::<&'static str>() {
-                            Err(format!("panicked in worker #{}: {}", thread_num, msg))
+                            anyhow::bail!("panicked in worker #{}: {}", thread_num, msg)
                         } else {
-                            Err(format!("panicked in worker #{}", thread_num))
+                            anyhow::bail!("panicked in worker #{}", thread_num)
                         }
                     });
 

cranelift/filetests/src/lib.rs

@@ -60,9 +60,6 @@ mod test_stack_maps;
 mod test_unwind;
 mod test_verifier;
 
-/// The result of running the test in a file.
-type TestResult = Result<time::Duration, String>;
-
 /// Main entry point for `clif-util test`.
 ///
 /// Take a list of filenames which can be either `.clif` files or directories.
@@ -72,7 +69,7 @@ type TestResult = Result<time::Duration, String>;
 /// Directories are scanned recursively for test cases ending in `.clif`. These test cases are
 /// executed on background threads.
 ///
-pub fn run(verbose: bool, report_times: bool, files: &[String]) -> TestResult {
+pub fn run(verbose: bool, report_times: bool, files: &[String]) -> anyhow::Result<time::Duration> {
     let mut runner = TestRunner::new(verbose, report_times);
 
     for path in files.iter().map(Path::new) {
@@ -98,7 +95,7 @@ pub fn run_passes(
     passes: &[String],
     target: &str,
     file: &str,
-) -> TestResult {
+) -> anyhow::Result<time::Duration> {
     let mut runner = TestRunner::new(verbose, /* report_times */ false);
 
     let path = Path::new(file);
@@ -119,7 +116,7 @@ pub fn run_passes(
 ///
 /// This function knows how to create all of the possible `test <foo>` commands that can appear in
 /// a `.clif` test file.
-fn new_subtest(parsed: &TestCommand) -> subtest::SubtestResult<Box<dyn subtest::SubTest>> {
+fn new_subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn subtest::SubTest>> {
     match parsed.command {
         "binemit" => test_binemit::subtest(parsed),
         "cat" => test_cat::subtest(parsed),
@@ -143,6 +140,15 @@ fn new_subtest(parsed: &TestCommand) -> subtest::SubtestResult<Box<dyn subtest::
         "stack_maps" => test_stack_maps::subtest(parsed),
         "unwind" => test_unwind::subtest(parsed),
         "verifier" => test_verifier::subtest(parsed),
-        _ => Err(format!("unknown test command '{}'", parsed.command)),
+        _ => anyhow::bail!("unknown test command '{}'", parsed.command),
     }
 }
+
+fn pretty_anyhow_error(
+    func: &cranelift_codegen::ir::Function,
+    isa: Option<&dyn cranelift_codegen::isa::TargetIsa>,
+    err: cranelift_codegen::CodegenError,
+) -> anyhow::Error {
+    let s = cranelift_codegen::print_errors::pretty_error(func, isa, err);
+    anyhow::anyhow!("{}", s)
+}

cranelift/filetests/src/runner.rs

@@ -5,7 +5,6 @@
 
 use crate::concurrent::{ConcurrentRunner, Reply};
 use crate::runone;
-use crate::TestResult;
 use cranelift_codegen::timing;
 use std::error::Error;
 use std::ffi::OsStr;
@@ -24,12 +23,12 @@ struct QueueEntry {
     state: State,
 }
 
-#[derive(PartialEq, Eq, Debug)]
+#[derive(Debug)]
 enum State {
     New,
     Queued,
     Running,
-    Done(TestResult),
+    Done(anyhow::Result<time::Duration>),
 }
 
 #[derive(PartialEq, Eq, Debug, Clone, Copy)]
@@ -205,7 +204,7 @@ impl TestRunner {
     /// Schedule any new jobs to run.
     fn schedule_jobs(&mut self) {
         for jobid in self.new_tests..self.tests.len() {
-            assert_eq!(self.tests[jobid].state, State::New);
+            assert!(matches!(self.tests[jobid].state, State::New));
             if let Some(ref mut conc) = self.threads {
                 // Queue test for concurrent execution.
                 self.tests[jobid].state = State::Queued;
@@ -228,7 +227,7 @@ impl TestRunner {
     /// Schedule any new job to run for the pass command.
     fn schedule_pass_job(&mut self, passes: &[String], target: &str) {
         self.tests[0].state = State::Running;
-        let result: Result<time::Duration, String>;
+        let result: anyhow::Result<time::Duration>;
 
         let specified_target = match target {
             "" => None,
@@ -240,8 +239,8 @@ impl TestRunner {
     }
 
     /// Report the end of a job.
-    fn finish_job(&mut self, jobid: usize, result: TestResult) {
+    fn finish_job(&mut self, jobid: usize, result: anyhow::Result<time::Duration>) {
-        assert_eq!(self.tests[jobid].state, State::Running);
+        assert!(matches!(self.tests[jobid].state, State::Running));
         if result.is_err() {
             self.errors += 1;
         }
@@ -257,7 +256,7 @@ impl TestRunner {
     fn handle_reply(&mut self, reply: Reply) {
         match reply {
             Reply::Starting { jobid, .. } => {
-                assert_eq!(self.tests[jobid].state, State::Queued);
+                assert!(matches!(self.tests[jobid].state, State::Queued));
                 self.tests[jobid].state = State::Running;
             }
             Reply::Done { jobid, result } => {
@@ -274,7 +273,7 @@ impl TestRunner {
                         self.tests.len()
                     );
                     for jobid in self.reported_tests..self.tests.len() {
-                        if self.tests[jobid].state == State::Running {
+                        if let State::Running = self.tests[jobid].state {
                             println!("slow: {}", self.tests[jobid]);
                         }
                     }
@@ -356,7 +355,7 @@ impl TestRunner {
     }
 
     /// Scan pushed directories for tests and run them.
-    pub fn run(&mut self) -> TestResult {
+    pub fn run(&mut self) -> anyhow::Result<time::Duration> {
         let started = time::Instant::now();
         self.scan_dirs(IsPass::NotPass);
         self.schedule_jobs();
@@ -366,13 +365,17 @@ impl TestRunner {
         println!("{} tests", self.tests.len());
         match self.errors {
             0 => Ok(started.elapsed()),
-            1 => Err("1 failure".to_string()),
+            1 => anyhow::bail!("1 failure"),
-            n => Err(format!("{} failures", n)),
+            n => anyhow::bail!("{} failures", n),
         }
     }
 
     /// Scan pushed directories for tests and run specified passes from commandline on them.
-    pub fn run_passes(&mut self, passes: &[String], target: &str) -> TestResult {
+    pub fn run_passes(
+        &mut self,
+        passes: &[String],
+        target: &str,
+    ) -> anyhow::Result<time::Duration> {
         let started = time::Instant::now();
         self.scan_dirs(IsPass::Pass);
         self.schedule_pass_job(passes, target);
@@ -381,8 +384,8 @@ impl TestRunner {
         println!("{} tests", self.tests.len());
         match self.errors {
             0 => Ok(started.elapsed()),
-            1 => Err("1 failure".to_string()),
+            1 => anyhow::bail!("1 failure"),
-            n => Err(format!("{} failures", n)),
+            n => anyhow::bail!("{} failures", n),
         }
     }
 }

cranelift/filetests/src/runone.rs

@@ -1,7 +1,8 @@
 //! Run the tests in a single test file.
 
-use crate::subtest::{Context, SubTest, SubtestResult};
+use crate::new_subtest;
-use crate::{new_subtest, TestResult};
+use crate::subtest::{Context, SubTest};
+use anyhow::Context as _;
 use cranelift_codegen::ir::Function;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::print_errors::pretty_verifier_error;
@@ -18,11 +19,16 @@ use std::time;
 /// Load `path` and run the test in it.
 ///
 /// If running this test causes a panic, it will propagate as normal.
-pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> TestResult {
+pub fn run(
+    path: &Path,
+    passes: Option<&[String]>,
+    target: Option<&str>,
+) -> anyhow::Result<time::Duration> {
     let _tt = timing::process_file();
     info!("---\nFile: {}", path.to_string_lossy());
     let started = time::Instant::now();
-    let buffer = fs::read_to_string(path).map_err(|e| e.to_string())?;
+    let buffer =
+        fs::read_to_string(path).with_context(|| format!("failed to read {}", path.display()))?;
     let options = ParseOptions {
         target,
         passes,
@@ -39,12 +45,14 @@ pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> Test
                 );
                 return Ok(started.elapsed());
             }
-            return Err(e.to_string());
+            return Err(e)
+                .context(format!("failed to parse {}", path.display()))
+                .into();
         }
     };
 
     if testfile.functions.is_empty() {
-        return Err("no functions found".to_string());
+        anyhow::bail!("no functions found");
     }
 
     // Parse the test commands.
@@ -52,7 +60,7 @@ pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> Test
         .commands
         .iter()
         .map(new_subtest)
-        .collect::<SubtestResult<Vec<_>>>()?;
+        .collect::<anyhow::Result<Vec<_>>>()?;
 
     // Flags to use for those tests that don't need an ISA.
     // This is the cumulative effect of all the `set` commands in the file.
@@ -71,7 +79,7 @@ pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> Test
     // Isolate the last test in the hope that this is the only mutating test.
     // If so, we can completely avoid cloning functions.
     let last_tuple = match tuples.pop() {
-        None => return Err("no test commands found".to_string()),
+        None => anyhow::bail!("no test commands found"),
         Some(t) => t,
     };
 
@@ -102,14 +110,14 @@ fn test_tuples<'a>(
     tests: &'a [Box<dyn SubTest>],
     isa_spec: &'a IsaSpec,
     no_isa_flags: &'a Flags,
-) -> SubtestResult<Vec<(&'a dyn SubTest, &'a Flags, Option<&'a dyn TargetIsa>)>> {
+) -> anyhow::Result<Vec<(&'a dyn SubTest, &'a Flags, Option<&'a dyn TargetIsa>)>> {
     let mut out = Vec::new();
     for test in tests {
         if test.needs_isa() {
             match *isa_spec {
                 IsaSpec::None(_) => {
                     // TODO: Generate a list of default ISAs.
-                    return Err(format!("test {} requires an ISA", test.name()));
+                    anyhow::bail!("test {} requires an ISA", test.name());
                 }
                 IsaSpec::Some(ref isas) => {
                     for isa in isas {
@@ -131,7 +139,7 @@ fn run_one_test<'a>(
     tuple: (&'a dyn SubTest, &'a Flags, Option<&'a dyn TargetIsa>),
     func: Cow<Function>,
     context: &mut Context<'a>,
-) -> SubtestResult<()> {
+) -> anyhow::Result<()> {
     let (test, flags, isa) = tuple;
     let name = format!("{}({})", test.name(), func.name);
     info!("Test: {} {}", name, isa.map_or("-", TargetIsa::name));
@@ -141,11 +149,12 @@ fn run_one_test<'a>(
 
     // Should we run the verifier before this test?
     if !context.verified && test.needs_verifier() {
-        verify_function(&func, context.flags_or_isa())
+        verify_function(&func, context.flags_or_isa()).map_err(|errors| {
-            .map_err(|errors| pretty_verifier_error(&func, isa, None, errors))?;
+            anyhow::anyhow!("{}", pretty_verifier_error(&func, isa, None, errors))
+        })?;
         context.verified = true;
     }
 
-    test.run(func, context)
+    test.run(func, context).context(test.name())?;
-        .map_err(|e| format!("{}:\n{}", name, e))
+    Ok(())
 }

cranelift/filetests/src/subtest.rs

@@ -1,5 +1,6 @@
 //! `SubTest` trait.
 
+use anyhow::Context as _;
 use cranelift_codegen::ir::Function;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::settings::{Flags, FlagsOrIsa};
@@ -7,8 +8,6 @@ use cranelift_reader::{Comment, Details};
 use filecheck::{Checker, CheckerBuilder, NO_VARIABLES};
 use std::borrow::Cow;
 
-pub type SubtestResult<T> = Result<T, String>;
-
 /// Context for running a test on a single function.
 pub struct Context<'a> {
     /// Comments from the preamble f the test file. These apply to all functions.
@@ -66,39 +65,39 @@ pub trait SubTest {
     }
 
     /// Run this test on `func`.
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()>;
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()>;
 }
 
 /// Run filecheck on `text`, using directives extracted from `context`.
-pub fn run_filecheck(text: &str, context: &Context) -> SubtestResult<()> {
+pub fn run_filecheck(text: &str, context: &Context) -> anyhow::Result<()> {
     let checker = build_filechecker(context)?;
     if checker
         .check(text, NO_VARIABLES)
-        .map_err(|e| format!("filecheck: {}", e))?
+        .context("filecheck failed")?
     {
         Ok(())
     } else {
         // Filecheck mismatch. Emit an explanation as output.
         let (_, explain) = checker
             .explain(text, NO_VARIABLES)
-            .map_err(|e| format!("explain: {}", e))?;
+            .context("filecheck explain failed")?;
-        Err(format!("filecheck failed:\n{}{}", checker, explain))
+        anyhow::bail!("filecheck failed:\n{}{}", checker, explain);
     }
 }
 
 /// Build a filechecker using the directives in the file preamble and the function's comments.
-pub fn build_filechecker(context: &Context) -> SubtestResult<Checker> {
+pub fn build_filechecker(context: &Context) -> anyhow::Result<Checker> {
     let mut builder = CheckerBuilder::new();
     // Preamble comments apply to all functions.
     for comment in context.preamble_comments {
         builder
             .directive(comment.text)
-            .map_err(|e| format!("filecheck: {}", e))?;
+            .context("filecheck directive failed")?;
     }
     for comment in &context.details.comments {
         builder
             .directive(comment.text)
-            .map_err(|e| format!("filecheck: {}", e))?;
+            .context("filecheck directive failed")?;
     }
     Ok(builder.finish())
 }

cranelift/filetests/src/test_binemit.rs

@@ -4,7 +4,7 @@
 //! functions and compares the results to the expected output.
 
 use crate::match_directive::match_directive;
-use crate::subtest::{Context, SubTest, SubtestResult};
+use crate::subtest::{Context, SubTest};
 use cranelift_codegen::binemit::{self, CodeInfo, CodeSink, RegDiversions};
 use cranelift_codegen::dbg::DisplayList;
 use cranelift_codegen::dominator_tree::DominatorTree;
@@ -12,7 +12,6 @@ use cranelift_codegen::flowgraph::ControlFlowGraph;
 use cranelift_codegen::ir;
 use cranelift_codegen::ir::entities::AnyEntity;
 use cranelift_codegen::isa;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_codegen::settings::OptLevel;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
@@ -21,10 +20,10 @@ use std::fmt::Write;
 
 struct TestBinEmit;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "binemit");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed)
     } else {
         Ok(Box::new(TestBinEmit))
     }
@@ -130,7 +129,7 @@ impl SubTest for TestBinEmit {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("binemit needs an ISA");
         let encinfo = isa.encoding_info();
         // TODO: Run a verifier pass over the code first to detect any bad encodings or missing/bad
@@ -187,7 +186,7 @@ impl SubTest for TestBinEmit {
         let mut domtree = DominatorTree::with_function(&func, &cfg);
         let CodeInfo { total_size, .. } =
             binemit::relax_branches(&mut func, &mut cfg, &mut domtree, isa)
-                .map_err(|e| pretty_error(&func, context.isa, e))?;
+                .map_err(|e| crate::pretty_anyhow_error(&func, context.isa, e))?;
 
         // Collect all of the 'bin:' directives on instructions.
         let mut bins = HashMap::new();
@@ -196,25 +195,26 @@ impl SubTest for TestBinEmit {
                 match comment.entity {
                     AnyEntity::Inst(inst) => {
                         if let Some(prev) = bins.insert(inst, want) {
-                            return Err(format!(
+                            anyhow::bail!(
                                 "multiple 'bin:' directives on {}: '{}' and '{}'",
                                 func.dfg.display_inst(inst, isa),
                                 prev,
                                 want
-                            ));
+                            );
                         }
                     }
                     _ => {
-                        return Err(format!(
+                        anyhow::bail!(
                             "'bin:' directive on non-inst {}: {}",
-                            comment.entity, comment.text
+                            comment.entity,
-                        ));
+                            comment.text
+                        );
                     }
                 }
             }
         }
         if bins.is_empty() {
-            return Err("No 'bin:' directives found".to_string());
+            anyhow::bail!("No 'bin:' directives found");
         }
 
         // Now emit all instructions.
@@ -265,26 +265,26 @@ impl SubTest for TestBinEmit {
                             .collect::<Vec<_>>();
 
                         if encodings.is_empty() {
-                            return Err(format!(
+                            anyhow::bail!(
                                 "No encodings found for: {}",
                                 func.dfg.display_inst(inst, isa)
-                            ));
+                            );
                         }
-                        return Err(format!(
+                        anyhow::bail!(
                             "No matching encodings for {} in {}",
                             func.dfg.display_inst(inst, isa),
                             DisplayList(&encodings),
-                        ));
+                        );
                     }
                     let have = sink.text.trim();
                     if have != want {
-                        return Err(format!(
+                        anyhow::bail!(
                             "Bad machine code for {}: {}\nWant: {}\nGot:  {}",
                             inst,
                             func.dfg.display_inst(inst, isa),
                             want,
                             have
-                        ));
+                        );
                     }
                 }
             }
@@ -312,10 +312,7 @@ impl SubTest for TestBinEmit {
         sink.end_codegen();
 
         if sink.offset != total_size {
-            return Err(format!(
+            anyhow::bail!("Expected code size {}, got {}", total_size, sink.offset);
-                "Expected code size {}, got {}",
-                total_size, sink.offset
-            ));
         }
 
         Ok(())
@@ -328,7 +325,7 @@ fn validate_location_annotations(
     inst: ir::Inst,
     isa: &dyn isa::TargetIsa,
     validate_inputs: bool,
-) -> SubtestResult<()> {
+) -> anyhow::Result<()> {
     let values = if validate_inputs {
         func.dfg.inst_args(inst)
     } else {
@@ -336,12 +333,11 @@ fn validate_location_annotations(
     };
 
     if let Some(&v) = values.iter().find(|&&v| !func.locations[v].is_assigned()) {
-        Err(format!(
+        anyhow::bail!(
             "Need register/stack slot annotation for {} in {}",
             v,
             func.dfg.display_inst(inst, isa)
-        ))
+        );
-    } else {
-        Ok(())
     }
+    Ok(())
 }

cranelift/filetests/src/test_cat.rs

@@ -1,6 +1,6 @@
 //! The `cat` subtest.
 
-use crate::subtest::{self, Context, SubTest, SubtestResult};
+use crate::subtest::{self, Context, SubTest};
 use cranelift_codegen::ir::Function;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
@@ -13,13 +13,12 @@ use std::borrow::Cow;
 /// The result is verified by filecheck.
 struct TestCat;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "cat");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestCat))
     }
+    Ok(Box::new(TestCat))
 }
 
 impl SubTest for TestCat {
@@ -31,7 +30,7 @@ impl SubTest for TestCat {
         false
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         subtest::run_filecheck(&func.display(context.isa).to_string(), context)
     }
 }

cranelift/filetests/src/test_compile.rs

@@ -2,25 +2,23 @@
 //!
 //! The `compile` test command runs each function through the full code generator pipeline
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::binemit::{self, CodeInfo};
 use cranelift_codegen::ir;
 use cranelift_codegen::isa;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use log::info;
 use std::borrow::Cow;
 
 struct TestCompile;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "compile");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestCompile))
     }
+    Ok(Box::new(TestCompile))
 }
 
 impl SubTest for TestCompile {
@@ -36,7 +34,7 @@ impl SubTest for TestCompile {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("compile needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
@@ -47,7 +45,7 @@ impl SubTest for TestCompile {
 
         let CodeInfo { total_size, .. } = comp_ctx
             .compile(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         info!(
             "Generated {} bytes of code:\n{}",
@@ -66,10 +64,7 @@ impl SubTest for TestCompile {
             );
 
             if sink.offset != total_size {
-                return Err(format!(
+                anyhow::bail!("Expected code size {}, got {}", total_size, sink.offset);
-                    "Expected code size {}, got {}",
-                    total_size, sink.offset
-                ));
             }
 
             // Run final code through filecheck.

cranelift/filetests/src/test_dce.rs

@@ -5,22 +5,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestDCE;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "dce");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestDCE))
     }
+    Ok(Box::new(TestDCE))
 }
 
 impl SubTest for TestDCE {
@@ -32,14 +30,14 @@ impl SubTest for TestDCE {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         comp_ctx.flowgraph();
         comp_ctx.compute_loop_analysis();
         comp_ctx
             .dce(context.flags_or_isa())
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(context.isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_domtree.rs

@@ -13,7 +13,7 @@
 //!
 
 use crate::match_directive::match_directive;
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen::dominator_tree::{DominatorTree, DominatorTreePreorder};
 use cranelift_codegen::flowgraph::ControlFlowGraph;
 use cranelift_codegen::ir::entities::AnyEntity;
@@ -25,13 +25,12 @@ use std::fmt::{self, Write};
 
 struct TestDomtree;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "domtree");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed)
-    } else {
-        Ok(Box::new(TestDomtree))
     }
+    Ok(Box::new(TestDomtree))
 }
 
 impl SubTest for TestDomtree {
@@ -40,7 +39,7 @@ impl SubTest for TestDomtree {
     }
 
     // Extract our own dominator tree from
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let func = func.borrow();
         let cfg = ControlFlowGraph::with_function(func);
         let domtree = DominatorTree::with_function(func, &cfg);
@@ -52,38 +51,42 @@ impl SubTest for TestDomtree {
                 let inst = match comment.entity {
                     AnyEntity::Inst(inst) => inst,
                     _ => {
-                        return Err(format!(
+                        anyhow::bail!(
                             "annotation on non-inst {}: {}",
-                            comment.entity, comment.text
+                            comment.entity,
-                        ));
+                            comment.text
+                        );
                     }
                 };
                 for src_block in tail.split_whitespace() {
                     let block = match context.details.map.lookup_str(src_block) {
                         Some(AnyEntity::Block(block)) => block,
-                        _ => return Err(format!("expected defined block, got {}", src_block)),
+                        _ => anyhow::bail!("expected defined block, got {}", src_block),
                     };
 
                     // Annotations say that `inst` is the idom of `block`.
                     if expected.insert(block, inst).is_some() {
-                        return Err(format!("multiple dominators for {}", src_block));
+                        anyhow::bail!("multiple dominators for {}", src_block);
                     }
 
                     // Compare to computed domtree.
                     match domtree.idom(block) {
                         Some(got_inst) if got_inst != inst => {
-                            return Err(format!(
+                            anyhow::bail!(
                                 "mismatching idoms for {}:\n\
                                  want: {}, got: {}",
-                                src_block, inst, got_inst
+                                src_block,
-                            ));
+                                inst,
+                                got_inst
+                            );
                         }
                         None => {
-                            return Err(format!(
+                            anyhow::bail!(
                                 "mismatching idoms for {}:\n\
                                  want: {}, got: unreachable",
-                                src_block, inst
+                                src_block,
-                            ));
+                                inst
+                            );
                         }
                         _ => {}
                     }
@@ -100,11 +103,12 @@ impl SubTest for TestDomtree {
             .filter(|block| !expected.contains_key(block))
         {
             if let Some(got_inst) = domtree.idom(block) {
-                return Err(format!(
+                anyhow::bail!(
                     "mismatching idoms for renumbered {}:\n\
                      want: unrechable, got: {}",
-                    block, got_inst
+                    block,
-                ));
+                    got_inst
+                );
             }
         }
 

cranelift/filetests/src/test_interpret.rs

@@ -3,7 +3,7 @@
 //! The `interpret` test command interprets each function on the host machine
 //! using [RunCommand](cranelift_reader::RunCommand)s.
 
-use crate::subtest::{Context, SubTest, SubtestResult};
+use crate::subtest::{Context, SubTest};
 use cranelift_codegen::{self, ir};
 use cranelift_interpreter::environment::Environment;
 use cranelift_interpreter::interpreter::{ControlFlow, Interpreter};
@@ -13,13 +13,12 @@ use std::borrow::Cow;
 
 struct TestInterpret;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "interpret");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestInterpret))
     }
+    Ok(Box::new(TestInterpret))
 }
 
 impl SubTest for TestInterpret {
@@ -35,26 +34,28 @@ impl SubTest for TestInterpret {
         false
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         for comment in context.details.comments.iter() {
-            if let Some(command) =
+            if let Some(command) = parse_run_command(comment.text, &func.signature)? {
-                parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())?
-            {
                 trace!("Parsed run command: {}", command);
 
                 let mut env = Environment::default();
                 env.add(func.name.to_string(), func.clone().into_owned());
                 let interpreter = Interpreter::new(env);
 
-                command.run(|func_name, args| {
+                command
+                    .run(|func_name, args| {
                         // Because we have stored function names with a leading %, we need to re-add it.
                         let func_name = &format!("%{}", func_name);
                         match interpreter.call_by_name(func_name, args) {
                             Ok(ControlFlow::Return(results)) => Ok(results),
-                        Ok(_) => panic!("Unexpected returned control flow--this is likely a bug."),
+                            Ok(_) => {
-                        Err(t) => Err(t.to_string()),
+                                panic!("Unexpected returned control flow--this is likely a bug.")
                             }
-                })?;
+                            Err(t) => Err(format!("unexpected trap: {:?}", t)),
+                        }
+                    })
+                    .map_err(|e| anyhow::anyhow!("{}", e))?;
             }
         }
         Ok(())

cranelift/filetests/src/test_legalizer.rs

@@ -3,22 +3,20 @@
 //! The `test legalizer` test command runs each function through `legalize_function()` and sends
 //! the result to filecheck.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestLegalizer;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "legalizer");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestLegalizer))
     }
+    Ok(Box::new(TestLegalizer))
 }
 
 impl SubTest for TestLegalizer {
@@ -34,14 +32,14 @@ impl SubTest for TestLegalizer {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
         let isa = context.isa.expect("legalizer needs an ISA");
 
         comp_ctx.compute_cfg();
         comp_ctx
             .legalize(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         let text = comp_ctx.func.display(Some(isa)).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_licm.rs

@@ -5,22 +5,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestLICM;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "licm");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestLICM))
     }
+    Ok(Box::new(TestLICM))
 }
 
 impl SubTest for TestLICM {
@@ -32,7 +30,7 @@ impl SubTest for TestLICM {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("LICM needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
@@ -40,7 +38,7 @@ impl SubTest for TestLICM {
         comp_ctx.compute_loop_analysis();
         comp_ctx
             .licm(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(context.isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_peepmatic.rs

@@ -1,20 +1,19 @@
 //! Test command for `peepmatic`-generated peephole optimizers.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestPreopt;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "peepmatic");
     if parsed.options.is_empty() {
         Ok(Box::new(TestPreopt))
     } else {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
     }
 }
 
@@ -31,14 +30,14 @@ impl SubTest for TestPreopt {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
         let isa = context.isa.expect("preopt needs an ISA");
 
         comp_ctx.compute_cfg();
         comp_ctx
             .preopt(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
         let text = &comp_ctx.func.display(isa).to_string();
         log::debug!("After peepmatic-based simple_preopt:\n{}", text);
 

cranelift/filetests/src/test_postopt.rs

@@ -2,22 +2,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestPostopt;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "postopt");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestPostopt))
     }
+    Ok(Box::new(TestPostopt))
 }
 
 impl SubTest for TestPostopt {
@@ -29,14 +27,14 @@ impl SubTest for TestPostopt {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
         let isa = context.isa.expect("postopt needs an ISA");
 
         comp_ctx.flowgraph();
         comp_ctx
             .postopt(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_preopt.rs

@@ -5,23 +5,21 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_preopt::optimize;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestPreopt;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "preopt");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestPreopt))
     }
+    Ok(Box::new(TestPreopt))
 }
 
 impl SubTest for TestPreopt {
@@ -37,12 +35,12 @@ impl SubTest for TestPreopt {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("compile needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         optimize(&mut comp_ctx, isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(context.isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_print_cfg.rs

@@ -5,7 +5,7 @@
 
 use std::borrow::Cow;
 
-use crate::subtest::{self, Context, SubTest, SubtestResult};
+use crate::subtest::{self, Context, SubTest};
 use cranelift_codegen::cfg_printer::CFGPrinter;
 use cranelift_codegen::ir::Function;
 use cranelift_reader::TestCommand;
@@ -13,13 +13,12 @@ use cranelift_reader::TestCommand;
 /// Object implementing the `test print-cfg` sub-test.
 struct TestPrintCfg;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "print-cfg");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestPrintCfg))
     }
+    Ok(Box::new(TestPrintCfg))
 }
 
 impl SubTest for TestPrintCfg {
@@ -31,7 +30,7 @@ impl SubTest for TestPrintCfg {
         false
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         subtest::run_filecheck(&CFGPrinter::new(&func).to_string(), context)
     }
 }

cranelift/filetests/src/test_regalloc.rs

@@ -5,22 +5,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestRegalloc;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "regalloc");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestRegalloc))
     }
+    Ok(Box::new(TestRegalloc))
 }
 
 impl SubTest for TestRegalloc {
@@ -36,7 +34,7 @@ impl SubTest for TestRegalloc {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("register allocator needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
@@ -44,11 +42,11 @@ impl SubTest for TestRegalloc {
         // TODO: Should we have an option to skip legalization?
         comp_ctx
             .legalize(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
         comp_ctx.compute_domtree();
         comp_ctx
             .regalloc(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         let text = comp_ctx.func.display(Some(isa)).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_rodata.rs

@@ -2,26 +2,24 @@
 //!
 //! The `rodata` test command runs each function through the full code generator pipeline
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::binemit::{self, CodeInfo};
 use cranelift_codegen::ir;
 use cranelift_codegen::ir::{Function, Value};
 use cranelift_codegen::isa::TargetIsa;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use log::info;
 use std::borrow::Cow;
 
 struct TestRodata;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "rodata");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestRodata))
     }
+    Ok(Box::new(TestRodata))
 }
 
 impl SubTest for TestRodata {
@@ -37,13 +35,13 @@ impl SubTest for TestRodata {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("rodata needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         let CodeInfo { total_size, .. } = comp_ctx
             .compile(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         info!(
             "Generated {} bytes of code:\n{}",

cranelift/filetests/src/test_run.rs

@@ -3,7 +3,7 @@
 //! The `run` test command compiles each function on the host machine and executes it
 
 use crate::function_runner::SingleFunctionCompiler;
-use crate::subtest::{Context, SubTest, SubtestResult};
+use crate::subtest::{Context, SubTest};
 use cranelift_codegen::ir;
 use cranelift_reader::parse_run_command;
 use cranelift_reader::TestCommand;
@@ -13,13 +13,12 @@ use target_lexicon::Architecture;
 
 struct TestRun;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "run");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestRun))
     }
+    Ok(Box::new(TestRun))
 }
 
 impl SubTest for TestRun {
@@ -35,7 +34,7 @@ impl SubTest for TestRun {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         // If this test requests to run on a completely different
         // architecture than the host platform then we skip it entirely,
         // since we won't be able to natively execute machine code.
@@ -50,9 +49,7 @@ impl SubTest for TestRun {
 
         let mut compiler = SingleFunctionCompiler::with_host_isa(context.flags.clone());
         for comment in context.details.comments.iter() {
-            if let Some(command) =
+            if let Some(command) = parse_run_command(comment.text, &func.signature)? {
-                parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())?
-            {
                 trace!("Parsed run command: {}", command);
 
                 // Note that here we're also explicitly ignoring `context.isa`,
@@ -60,10 +57,10 @@ impl SubTest for TestRun {
                 // host ISA no matter what here, so the ISA listed in the file
                 // is only used as a filter to not run into situations like
                 // running x86_64 code on aarch64 platforms.
-                let compiled_fn = compiler
+                let compiled_fn = compiler.compile(func.clone().into_owned())?;
-                    .compile(func.clone().into_owned())
+                command
-                    .map_err(|e| format!("{:?}", e))?;
+                    .run(|_, args| Ok(compiled_fn.call(args)))
-                command.run(|_, args| Ok(compiled_fn.call(args)))?;
+                    .map_err(|s| anyhow::anyhow!("{}", s))?;
             }
         }
         Ok(())

cranelift/filetests/src/test_safepoint.rs

@@ -1,18 +1,16 @@
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestSafepoint;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "safepoint");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestSafepoint))
     }
+    Ok(Box::new(TestSafepoint))
 }
 
 impl SubTest for TestSafepoint {
@@ -20,18 +18,18 @@ impl SubTest for TestSafepoint {
         "safepoint"
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         let isa = context.isa.expect("register allocator needs an ISA");
         comp_ctx.compute_cfg();
         comp_ctx
             .legalize(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
         comp_ctx.compute_domtree();
         comp_ctx
             .regalloc(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         let text = comp_ctx.func.display(context.isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_shrink.rs

@@ -5,22 +5,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestShrink;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "shrink");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestShrink))
     }
+    Ok(Box::new(TestShrink))
 }
 
 impl SubTest for TestShrink {
@@ -32,13 +30,13 @@ impl SubTest for TestShrink {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("shrink needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         comp_ctx
             .shrink_instructions(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_simple_gvn.rs

@@ -5,22 +5,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestSimpleGVN;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "simple-gvn");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestSimpleGVN))
     }
+    Ok(Box::new(TestSimpleGVN))
 }
 
 impl SubTest for TestSimpleGVN {
@@ -32,13 +30,13 @@ impl SubTest for TestSimpleGVN {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         comp_ctx.flowgraph();
         comp_ctx
             .simple_gvn(context.flags_or_isa())
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, Into::into(e)))?;
 
         let text = comp_ctx.func.display(context.isa).to_string();
         run_filecheck(&text, context)

cranelift/filetests/src/test_simple_preopt.rs

@@ -2,22 +2,20 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen;
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 
 struct TestSimplePreopt;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "simple_preopt");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestSimplePreopt))
     }
+    Ok(Box::new(TestSimplePreopt))
 }
 
 impl SubTest for TestSimplePreopt {
@@ -29,14 +27,14 @@ impl SubTest for TestSimplePreopt {
         true
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
         let isa = context.isa.expect("preopt needs an ISA");
 
         comp_ctx.compute_cfg();
         comp_ctx
             .preopt(isa)
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, Into::into(e)))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
         let text = &comp_ctx.func.display(isa).to_string();
         log::debug!("After simple_preopt:\n{}", text);
 

cranelift/filetests/src/test_stack_maps.rs

@@ -1,21 +1,19 @@
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen::binemit::{self, Addend, CodeOffset, CodeSink, Reloc, StackMap};
 use cranelift_codegen::ir::*;
 use cranelift_codegen::isa::TargetIsa;
-use cranelift_codegen::print_errors::pretty_error;
 use cranelift_reader::TestCommand;
 use std::borrow::Cow;
 use std::fmt::Write;
 
 struct TestStackMaps;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "stack_maps");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestStackMaps))
     }
+    Ok(Box::new(TestStackMaps))
 }
 
 impl SubTest for TestStackMaps {
@@ -23,12 +21,12 @@ impl SubTest for TestStackMaps {
         "stack_maps"
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
         comp_ctx
             .compile(context.isa.expect("`test stack_maps` requires an isa"))
-            .map_err(|e| pretty_error(&comp_ctx.func, context.isa, e))?;
+            .map_err(|e| crate::pretty_anyhow_error(&comp_ctx.func, context.isa, e))?;
 
         let mut sink = TestStackMapsSink::default();
         binemit::emit_function(

cranelift/filetests/src/test_unwind.rs

@@ -3,7 +3,7 @@
 //! The `unwind` test command runs each function through the full code generator pipeline.
 #![cfg_attr(feature = "cargo-clippy", allow(clippy::cast_ptr_alignment))]
 
-use crate::subtest::{run_filecheck, Context, SubTest, SubtestResult};
+use crate::subtest::{run_filecheck, Context, SubTest};
 use cranelift_codegen::{self, ir, isa::unwind::UnwindInfo};
 use cranelift_reader::TestCommand;
 use gimli::{
@@ -14,13 +14,12 @@ use std::borrow::Cow;
 
 struct TestUnwind;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "unwind");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestUnwind))
     }
+    Ok(Box::new(TestUnwind))
 }
 
 impl SubTest for TestUnwind {
@@ -36,7 +35,7 @@ impl SubTest for TestUnwind {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
         let isa = context.isa.expect("unwind needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 

cranelift/filetests/src/test_verifier.rs

@@ -10,7 +10,7 @@
 //! containing the substring "jump to non-existent block".
 
 use crate::match_directive::match_directive;
-use crate::subtest::{Context, SubTest, SubtestResult};
+use crate::subtest::{Context, SubTest};
 use cranelift_codegen::ir::Function;
 use cranelift_codegen::verify_function;
 use cranelift_reader::TestCommand;
@@ -19,13 +19,12 @@ use std::fmt::Write;
 
 struct TestVerifier;
 
-pub fn subtest(parsed: &TestCommand) -> SubtestResult<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "verifier");
     if !parsed.options.is_empty() {
-        Err(format!("No options allowed on {}", parsed))
+        anyhow::bail!("No options allowed on {}", parsed);
-    } else {
-        Ok(Box::new(TestVerifier))
     }
+    Ok(Box::new(TestVerifier))
 }
 
 impl SubTest for TestVerifier {
@@ -38,7 +37,7 @@ impl SubTest for TestVerifier {
         false
     }
 
-    fn run(&self, func: Cow<Function>, context: &Context) -> SubtestResult<()> {
+    fn run(&self, func: Cow<Function>, context: &Context) -> anyhow::Result<()> {
         let func = func.borrow();
 
         // Scan source annotations for "error:" directives.
@@ -52,10 +51,10 @@ impl SubTest for TestVerifier {
 
         match verify_function(func, context.flags_or_isa()) {
             Ok(()) if expected.is_empty() => Ok(()),
-            Ok(()) => Err(format!("passed, but expected errors: {:?}", expected)),
+            Ok(()) => anyhow::bail!("passed, but expected errors: {:?}", expected),
 
             Err(ref errors) if expected.is_empty() => {
-                Err(format!("expected no error, but got:\n{}", errors))
+                anyhow::bail!("expected no error, but got:\n{}", errors);
             }
 
             Err(errors) => {
@@ -86,7 +85,7 @@ impl SubTest for TestVerifier {
                 if msg.is_empty() {
                     Ok(())
                 } else {
-                    Err(msg)
+                    anyhow::bail!("{}", msg);
                 }
             }
         }

cranelift/reader/src/error.rs

@@ -33,6 +33,8 @@ impl fmt::Display for ParseError {
     }
 }
 
+impl std::error::Error for ParseError {}
+
 /// Result of a parser operation. The `ParseError` variant includes a location.
 pub type ParseResult<T> = Result<T, ParseError>;
 

cranelift/src/bugpoint.rs

@@ -2,6 +2,7 @@
 
 use crate::disasm::{PrintRelocs, PrintStackMaps, PrintTraps};
 use crate::utils::{parse_sets_and_triple, read_to_string};
+use anyhow::{Context as _, Result};
 use cranelift_codegen::cursor::{Cursor, FuncCursor};
 use cranelift_codegen::flowgraph::ControlFlowGraph;
 use cranelift_codegen::ir::types::{F32, F64};
@@ -13,25 +14,37 @@ use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::Context;
 use cranelift_entity::PrimaryMap;
 use cranelift_reader::{parse_test, ParseOptions};
-use std::collections::HashMap;
-use std::path::Path;
-
 use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle};
+use std::collections::HashMap;
+use std::path::PathBuf;
+use structopt::StructOpt;
 
-pub fn run(
+/// Reduce size of clif file causing panic during compilation.
-    filename: &str,
+#[derive(StructOpt)]
-    flag_set: &[String],
+pub struct Options {
-    flag_isa: &str,
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    file: PathBuf,
+
+    /// Configure Cranelift settings
+    #[structopt(long("set"))]
+    settings: Vec<String>,
+
+    /// Specify the target architecture.
+    target: String,
+
+    /// Be more verbose
+    #[structopt(short = "v", long = "verbose")]
     verbose: bool,
-) -> Result<(), String> {
+}
-    let parsed = parse_sets_and_triple(flag_set, flag_isa)?;
+
+pub fn run(options: &Options) -> Result<()> {
+    let parsed = parse_sets_and_triple(&options.settings, &options.target)?;
     let fisa = parsed.as_fisa();
 
-    let path = Path::new(&filename).to_path_buf();
+    let buffer = read_to_string(&options.file)?;
-
+    let test_file = parse_test(&buffer, ParseOptions::default())
-    let buffer = read_to_string(&path).map_err(|e| format!("{}: {}", filename, e))?;
+        .with_context(|| format!("failed to parse {}", options.file.display()))?;
-    let test_file =
-        parse_test(&buffer, ParseOptions::default()).map_err(|e| format!("{}: {}", filename, e))?;
 
     // If we have an isa from the command-line, use that. Otherwise if the
     // file contains a unique isa, use that.
@@ -40,7 +53,7 @@ pub fn run(
     } else if let Some(isa) = test_file.isa_spec.unique_isa() {
         isa
     } else {
-        return Err(String::from("compilation requires a target isa"));
+        anyhow::bail!("compilation requires a target isa");
     };
 
     std::env::set_var("RUST_BACKTRACE", "0"); // Disable backtraces to reduce verbosity
@@ -48,7 +61,7 @@ pub fn run(
     for (func, _) in test_file.functions {
         let (orig_block_count, orig_inst_count) = (block_count(&func), inst_count(&func));
 
-        match reduce(isa, func, verbose) {
+        match reduce(isa, func, options.verbose) {
             Ok((func, crash_msg)) => {
                 println!("Crash message: {}", crash_msg);
                 println!("\n{}", func);
@@ -833,20 +846,11 @@ fn try_resolve_aliases(context: &mut CrashCheckContext, func: &mut Function) {
     }
 }
 
-fn reduce(
+fn reduce(isa: &dyn TargetIsa, mut func: Function, verbose: bool) -> Result<(Function, String)> {
-    isa: &dyn TargetIsa,
-    mut func: Function,
-    verbose: bool,
-) -> Result<(Function, String), String> {
     let mut context = CrashCheckContext::new(isa);
 
-    match context.check_for_crash(&func) {
+    if let CheckResult::Succeed = context.check_for_crash(&func) {
-        CheckResult::Succeed => {
+        anyhow::bail!("Given function compiled successfully or gave a verifier error.");
-            return Err(
-                "Given function compiled successfully or gave a verifier error.".to_string(),
-            );
-        }
-        CheckResult::Crash(_) => {}
     }
 
     try_resolve_aliases(&mut context, &mut func);

cranelift/src/cat.rs

@@ -4,11 +4,26 @@
 //! normalizing formatting and removing comments.
 
 use crate::utils::read_to_string;
-use crate::CommandResult;
+use anyhow::{Context, Result};
 use cranelift_reader::parse_functions;
+use std::path::{Path, PathBuf};
+use structopt::StructOpt;
 
-pub fn run(files: &[String]) -> CommandResult {
+/// Outputs .clif file
-    for (i, f) in files.iter().enumerate() {
+#[derive(StructOpt)]
+pub struct Options {
+    /// Specify input file(s) to be used. Use '-' for stdin.
+    #[structopt(required(true), parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    crate::handle_debug_flag(options.debug);
+    for (i, f) in options.files.iter().enumerate() {
         if i != 0 {
             println!();
         }
@@ -17,9 +32,10 @@ pub fn run(files: &[String]) -> CommandResult {
     Ok(())
 }
 
-fn cat_one(filename: &str) -> CommandResult {
+fn cat_one(path: &Path) -> Result<()> {
-    let buffer = read_to_string(&filename).map_err(|e| format!("{}: {}", filename, e))?;
+    let buffer = read_to_string(path)?;
-    let items = parse_functions(&buffer).map_err(|e| format!("{}: {}", filename, e))?;
+    let items =
+        parse_functions(&buffer).with_context(|| format!("failed to parse {}", path.display()))?;
 
     for (idx, func) in items.into_iter().enumerate() {
         if idx != 0 {

cranelift/src/clif-util.rs

@@ -13,12 +13,9 @@
     )
 )]
 
-use clap::{arg_enum, App, Arg, SubCommand};
 use cranelift_codegen::dbg::LOG_FILENAME_PREFIX;
-use cranelift_codegen::VERSION;
+use std::{option::Option, path::PathBuf};
-use std::io::{self, Write};
+use structopt::StructOpt;
-use std::option::Option;
-use std::process;
 
 mod bugpoint;
 mod cat;
@@ -27,9 +24,10 @@ mod disasm;
 mod interpret;
 mod print_cfg;
 mod run;
+mod utils;
+
 #[cfg(feature = "souper-harvest")]
 mod souper_harvest;
-mod utils;
 
 #[cfg(feature = "peepmatic-souper")]
 mod souper_to_peepmatic;
@@ -37,160 +35,6 @@ mod souper_to_peepmatic;
 #[cfg(feature = "wasm")]
 mod wasm;
 
-/// A command either succeeds or fails with an error message.
-pub type CommandResult = Result<(), String>;
-
-fn add_input_file_arg<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("file")
-        .default_value("-")
-        .multiple(true)
-        .value_name("file")
-        .help("Specify file(s) to be used for test. Defaults to reading from stdin.")
-}
-
-fn add_single_input_file_arg<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("single-file")
-        .required(true)
-        .value_name("single-file")
-        .help("Specify a file to be used. Use '-' for stdin.")
-}
-
-fn add_output_arg<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("output")
-        .required(true)
-        .default_value("-")
-        .value_name("output")
-        .short("o")
-        .help("Specify output file to be used. Use '-' for stdout.")
-}
-
-fn add_pass_arg<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("pass")
-        .required(true)
-        .multiple(true)
-        .value_name("pass")
-        .help("Specify pass(s) to be run on test file")
-}
-
-fn add_verbose_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("verbose").short("v").help("Be more verbose")
-}
-
-arg_enum! {
-    #[derive(Clone, Copy, Debug, PartialEq)]
-    pub enum UseTerminalColor {
-        Auto,
-        Never,
-        Always
-    }
-}
-
-fn add_color<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("color")
-        .long("color")
-        .possible_values(&UseTerminalColor::variants())
-        .takes_value(true)
-        .multiple(false)
-        .default_value("auto")
-        .case_insensitive(true)
-        .help("Use colors in output")
-}
-
-fn add_time_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("time-passes")
-        .short("T")
-        .help("Print pass timing report for test")
-}
-
-fn add_size_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("print-size")
-        .short("X")
-        .help("Print bytecode size")
-}
-
-fn add_disasm_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("disasm")
-        .long("disasm")
-        .short("D")
-        .help("Print machine code disassembly")
-}
-
-fn add_set_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("set")
-        .long("set")
-        .takes_value(true)
-        .multiple(true)
-        .help("Configure Cranelift settings")
-}
-
-fn add_target_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("target")
-        .takes_value(true)
-        .long("target")
-        .help("Specify the Cranelift target")
-}
-
-fn add_print_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("print")
-        .short("p")
-        .help("Print the resulting Cranelift IR")
-}
-
-fn add_debug_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("debug")
-        .short("d")
-        .help("Enable debug output on stderr/stdout")
-}
-
-fn add_just_decode_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("just-decode")
-        .short("t")
-        .help("Just decode into Cranelift IR")
-}
-
-fn add_check_translation_flag<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("check-translation")
-        .short("c")
-        .help("Just checks the correctness of Cranelift IR translated from WebAssembly")
-}
-
-fn add_value_ranges<'a>() -> clap::Arg<'a, 'a> {
-    Arg::with_name("value-ranges")
-        .long("value-ranges")
-        .help("Display values ranges and their locations")
-}
-
-/// Returns a vector of clap value options and changes these options into a vector of strings
-fn get_vec(argument_vec: Option<clap::Values>) -> Vec<String> {
-    let mut ret_vec: Vec<String> = Vec::new();
-    if let Some(clap_vec) = argument_vec {
-        for val in clap_vec {
-            ret_vec.push(val.to_string());
-        }
-    }
-
-    ret_vec
-}
-
-fn add_wasm_or_compile<'a>(cmd: &str) -> clap::App<'a, 'a> {
-    let about_str = match cmd {
-        "wasm" => "Compiles Wasm binary/text into Cranelift IR and then into target language",
-        "compile" => "Compiles Cranelift IR into target language",
-        _ => panic!("Invalid command"),
-    };
-
-    SubCommand::with_name(cmd)
-        .about(about_str)
-        .arg(add_verbose_flag())
-        .arg(add_print_flag())
-        .arg(add_time_flag())
-        .arg(add_disasm_flag())
-        .arg(add_set_flag())
-        .arg(add_target_flag())
-        .arg(add_input_file_arg())
-        .arg(add_debug_flag())
-}
-
 fn handle_debug_flag(debug: bool) {
     if debug {
         pretty_env_logger::init();
@@ -199,239 +43,139 @@ fn handle_debug_flag(debug: bool) {
     }
 }
 
-fn main() {
+/// Cranelift code generator utility.
-    let app_cmds = App::new("Cranelift code generator utility")
+#[derive(StructOpt)]
-        .version(VERSION)
+enum Commands {
-        .subcommand(
+    Test(TestOptions),
-            SubCommand::with_name("test")
+    Run(run::Options),
-                .about("Run Cranelift tests")
+    Interpret(interpret::Options),
-                .arg(add_verbose_flag())
+    Cat(cat::Options),
-                .arg(add_time_flag())
+    PrintCfg(print_cfg::Options),
-                .arg(add_input_file_arg())
+    Compile(compile::Options),
-                .arg(add_debug_flag()),
+    Pass(PassOptions),
-        )
+    Bugpoint(bugpoint::Options),
-        .subcommand(
-            SubCommand::with_name("run")
-                .about("Execute CLIF code and verify with test expressions")
-                .arg(add_verbose_flag())
-                .arg(add_input_file_arg())
-                .arg(add_debug_flag()),
-        )
-        .subcommand(
-            SubCommand::with_name("interpret")
-                .about("Interpret CLIF code")
-                .arg(add_verbose_flag())
-                .arg(add_input_file_arg())
-                .arg(add_debug_flag()),
-        )
-        .subcommand(
-            SubCommand::with_name("cat")
-                .about("Outputs .clif file")
-                .arg(add_input_file_arg())
-                .arg(add_debug_flag()),
-        )
-        .subcommand(
-            SubCommand::with_name("print-cfg")
-                .about("Prints out cfg in dot format")
-                .arg(add_input_file_arg())
-                .arg(add_debug_flag()),
-        )
-        .subcommand(add_wasm_or_compile("compile"))
-        .subcommand(
-            add_wasm_or_compile("wasm")
-                .arg(add_size_flag())
-                .arg(add_just_decode_flag())
-                .arg(add_check_translation_flag())
-                .arg(add_value_ranges())
-                .arg(add_color()),
-        )
-        .subcommand(
-            SubCommand::with_name("pass")
-                .about("Run specified pass(s) on an input file.")
-                .arg(add_single_input_file_arg())
-                .arg(add_target_flag())
-                .arg(add_pass_arg())
-                .arg(add_debug_flag())
-                .arg(add_time_flag()),
-        )
-        .subcommand(
-            SubCommand::with_name("bugpoint")
-                .about("Reduce size of clif file causing panic during compilation.")
-                .arg(add_single_input_file_arg())
-                .arg(add_set_flag())
-                .arg(add_target_flag())
-                .arg(add_verbose_flag()),
-        )
-        .subcommand(
-            SubCommand::with_name("souper-to-peepmatic")
-                .about("Convert Souper optimizations into Peepmatic DSL.")
-                .arg(add_single_input_file_arg())
-                .arg(add_output_arg()),
-        )
-        .subcommand(
-            SubCommand::with_name("souper-harvest")
-                .arg(add_single_input_file_arg())
-                .arg(add_output_arg())
-                .arg(add_target_flag())
-                .arg(add_set_flag()),
-        );
-
-    let res_util = match app_cmds.get_matches().subcommand() {
-        ("cat", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-            cat::run(&get_vec(rest_cmd.values_of("file")))
-        }
-        ("test", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-            cranelift_filetests::run(
-                rest_cmd.is_present("verbose"),
-                rest_cmd.is_present("time-passes"),
-                &get_vec(rest_cmd.values_of("file")),
-            )
-            .map(|_time| ())
-        }
-        ("run", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-            run::run(
-                get_vec(rest_cmd.values_of("file")),
-                rest_cmd.is_present("verbose"),
-            )
-            .map(|_time| ())
-        }
-        ("interpret", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-            interpret::run(
-                get_vec(rest_cmd.values_of("file")),
-                rest_cmd.is_present("verbose"),
-            )
-            .map(|_time| ())
-        }
-        ("pass", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-
-            let mut target_val: &str = "";
-            if let Some(clap_target) = rest_cmd.value_of("target") {
-                target_val = clap_target;
-            }
-
-            // Can be unwrapped because 'single-file' is required
-            cranelift_filetests::run_passes(
-                rest_cmd.is_present("verbose"),
-                rest_cmd.is_present("time-passes"),
-                &get_vec(rest_cmd.values_of("pass")),
-                target_val,
-                rest_cmd.value_of("single-file").unwrap(),
-            )
-            .map(|_time| ())
-        }
-        ("print-cfg", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-            print_cfg::run(&get_vec(rest_cmd.values_of("file")))
-        }
-        ("compile", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
-
-            let mut target_val: &str = "";
-            if let Some(clap_target) = rest_cmd.value_of("target") {
-                target_val = clap_target;
-            }
-
-            compile::run(
-                get_vec(rest_cmd.values_of("file")),
-                rest_cmd.is_present("print"),
-                rest_cmd.is_present("disasm"),
-                rest_cmd.is_present("time-passes"),
-                &get_vec(rest_cmd.values_of("set")),
-                target_val,
-            )
-        }
-        ("wasm", Some(rest_cmd)) => {
-            handle_debug_flag(rest_cmd.is_present("debug"));
 
     #[cfg(feature = "wasm")]
-            let result = {
+    Wasm(wasm::Options),
-                let mut target_val: &str = "";
-                if let Some(clap_target) = rest_cmd.value_of("target") {
-                    target_val = clap_target;
-                }
-
-                wasm::run(
-                    get_vec(rest_cmd.values_of("file")),
-                    rest_cmd.value_of("color").unwrap().parse().unwrap(),
-                    rest_cmd.is_present("verbose"),
-                    rest_cmd.is_present("just-decode"),
-                    rest_cmd.is_present("check-translation"),
-                    rest_cmd.is_present("print"),
-                    rest_cmd.is_present("disasm"),
-                    &get_vec(rest_cmd.values_of("set")),
-                    target_val,
-                    rest_cmd.is_present("print-size"),
-                    rest_cmd.is_present("time-passes"),
-                    rest_cmd.is_present("value-ranges"),
-                )
-            };
-
     #[cfg(not(feature = "wasm"))]
-            let result = Err("Error: clif-util was compiled without wasm support.".to_owned());
+    Wasm(CompiledWithoutSupportOptions),
 
-            result
-        }
-        ("bugpoint", Some(rest_cmd)) => {
-            let mut target_val: &str = "";
-            if let Some(clap_target) = rest_cmd.value_of("target") {
-                target_val = clap_target;
-            }
-
-            bugpoint::run(
-                rest_cmd.value_of("single-file").unwrap(),
-                &get_vec(rest_cmd.values_of("set")),
-                target_val,
-                rest_cmd.is_present("verbose"),
-            )
-        }
-        ("souper-to-peepmatic", Some(rest_cmd)) => {
     #[cfg(feature = "peepmatic-souper")]
-            {
+    SouperToPeepmatic(souper_to_peepmatic::Options),
-                use std::path::Path;
-                souper_to_peepmatic::run(
-                    Path::new(rest_cmd.value_of("single-file").unwrap()),
-                    Path::new(rest_cmd.value_of("output").unwrap()),
-                )
-            }
     #[cfg(not(feature = "peepmatic-souper"))]
-            {
+    SouperToPeepmatic(CompiledWithoutSupportOptions),
-                Err(
+
-                    "Error: clif-util was compiled without support for the `souper-to-peepmatic` \
-                     subcommand"
-                        .into(),
-                )
-            }
-        }
-        ("souper-harvest", Some(rest_cmd)) => {
     #[cfg(feature = "souper-harvest")]
-            {
+    SouperHarvest(souper_harvest::Options),
-                souper_harvest::run(
-                    rest_cmd.value_of("target").unwrap_or_default(),
-                    rest_cmd.value_of("single-file").unwrap(),
-                    rest_cmd.value_of("output").unwrap(),
-                    &get_vec(rest_cmd.values_of("set")),
-                )
-            }
-
     #[cfg(not(feature = "souper-harvest"))]
-            {
+    SouperHarvest(CompiledWithoutSupportOptions),
-                Err("clif-util was compiled without `souper-harvest` support".into())
+}
-            }
+
-        }
+/// Run Cranelift tests
-        _ => Err("Invalid subcommand.".to_owned()),
+#[derive(StructOpt)]
-    };
+struct TestOptions {
-
+    /// Be more verbose
-    if let Err(mut msg) = res_util {
+    #[structopt(short = "v", long = "verbose")]
-        if !msg.ends_with('\n') {
+    verbose: bool,
-            msg.push('\n');
+
-        }
+    /// Print pass timing report for test
-        io::stdout().flush().expect("flushing stdout");
+    #[structopt(short = "T")]
-        io::stderr().write_all(msg.as_bytes()).unwrap();
+    time_passes: bool,
-        process::exit(1);
+
-    }
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(required(true), parse(from_os_str))]
+    files: Vec<PathBuf>,
+}
+
+/// Run specified pass(es) on an input file.
+#[derive(StructOpt)]
+struct PassOptions {
+    /// Be more verbose
+    #[structopt(short = "v", long = "verbose")]
+    verbose: bool,
+
+    /// Print pass timing report for test
+    #[structopt(short = "T")]
+    time_passes: bool,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    file: PathBuf,
+
+    /// Specify the target architecture.
+    target: String,
+
+    /// Specify pass(es) to be run on the input file
+    #[structopt(required(true))]
+    passes: Vec<String>,
+}
+
+/// (Compiled without support for this subcommand)
+#[derive(StructOpt)]
+struct CompiledWithoutSupportOptions {}
+
+fn main() -> anyhow::Result<()> {
+    match Commands::from_args() {
+        Commands::Cat(c) => cat::run(&c)?,
+        Commands::Run(r) => run::run(&r)?,
+        Commands::Interpret(i) => interpret::run(&i)?,
+        Commands::PrintCfg(p) => print_cfg::run(&p)?,
+        Commands::Compile(c) => compile::run(&c)?,
+        Commands::Bugpoint(b) => bugpoint::run(&b)?,
+
+        #[cfg(feature = "wasm")]
+        Commands::Wasm(w) => wasm::run(&w)?,
+        #[cfg(not(feature = "wasm"))]
+        Commands::Wasm(_) => anyhow::bail!("Error: clif-util was compiled without wasm support."),
+
+        #[cfg(feature = "peepmatic-souper")]
+        Commands::SouperToPeepmatic(s) => souper_to_peepmatic::run(&s)?,
+        #[cfg(not(feature = "peepmatic-souper"))]
+        Commands::SouperToPeepmatic(_) => anyhow::bail!(
+            "Error: clif-util was compiled without support for the `souper-to-peepmatic` \
+             subcommand",
+        ),
+
+        #[cfg(feature = "souper-harvest")]
+        Commands::SouperHarvest(s) => souper_harvest::run(&s)?,
+        #[cfg(not(feature = "souper-harvest"))]
+        Commands::SouperHarvest(_) => anyhow::bail!(
+            "Error: clif-util was compiled without support for the `souper-harvest` \
+             subcommand",
+        ),
+
+        Commands::Test(t) => {
+            handle_debug_flag(t.debug);
+            cranelift_filetests::run(
+                t.verbose,
+                t.time_passes,
+                &t.files
+                    .iter()
+                    .map(|f| f.display().to_string())
+                    .collect::<Vec<_>>(),
+            )
+            .map_err(|s| anyhow::anyhow!("{}", s))?;
+        }
+        Commands::Pass(p) => {
+            handle_debug_flag(p.debug);
+            cranelift_filetests::run_passes(
+                p.verbose,
+                p.time_passes,
+                &p.passes,
+                &p.target,
+                &p.file.display().to_string(),
+            )
+            .map_err(|s| anyhow::anyhow!("{}", s))?;
+        }
+    }
+
+    Ok(())
 }

cranelift/src/compile.rs

@@ -2,6 +2,7 @@
 
 use crate::disasm::{print_all, PrintRelocs, PrintStackMaps, PrintTraps};
 use crate::utils::{parse_sets_and_triple, read_to_string};
+use anyhow::{Context as _, Result};
 use cranelift_codegen::print_errors::pretty_error;
 use cranelift_codegen::settings::FlagsOrIsa;
 use cranelift_codegen::timing;
@@ -9,56 +10,67 @@ use cranelift_codegen::Context;
 use cranelift_reader::{parse_test, ParseOptions};
 use std::path::Path;
 use std::path::PathBuf;
+use structopt::StructOpt;
 
-pub fn run(
+/// Compiles Cranelift IR into target language
-    files: Vec<String>,
+#[derive(StructOpt)]
-    flag_print: bool,
+pub struct Options {
-    flag_disasm: bool,
+    /// Print the resulting Cranelift IR
-    flag_report_times: bool,
+    #[structopt(short("p"))]
-    flag_set: &[String],
+    print: bool,
-    flag_isa: &str,
-) -> Result<(), String> {
-    let parsed = parse_sets_and_triple(flag_set, flag_isa)?;
 
-    for filename in files {
+    /// Print pass timing report
-        let path = Path::new(&filename);
+    #[structopt(short("T"))]
+    report_times: bool,
+
+    /// Print machine code disassembly
+    #[structopt(short("D"), long("disasm"))]
+    disasm: bool,
+
+    /// Configure Cranelift settings
+    #[structopt(long("set"))]
+    settings: Vec<String>,
+
+    /// Specify the Cranelift target
+    #[structopt(long("target"))]
+    target: String,
+
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    crate::handle_debug_flag(options.debug);
+    let parsed = parse_sets_and_triple(&options.settings, &options.target)?;
+    for path in &options.files {
         let name = String::from(path.as_os_str().to_string_lossy());
-        handle_module(
+        handle_module(options, path, &name, parsed.as_fisa())?;
-            flag_print,
-            flag_disasm,
-            flag_report_times,
-            &path.to_path_buf(),
-            &name,
-            parsed.as_fisa(),
-        )?;
     }
     Ok(())
 }
 
-fn handle_module(
+fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) -> Result<()> {
-    flag_print: bool,
+    let buffer = read_to_string(&path)?;
-    flag_disasm: bool,
+    let test_file = parse_test(&buffer, ParseOptions::default())
-    flag_report_times: bool,
+        .with_context(|| format!("failed to parse {}", name))?;
-    path: &PathBuf,
-    name: &str,
-    fisa: FlagsOrIsa,
-) -> Result<(), String> {
-    let buffer = read_to_string(&path).map_err(|e| format!("{}: {}", name, e))?;
-    let test_file =
-        parse_test(&buffer, ParseOptions::default()).map_err(|e| format!("{}: {}", name, e))?;
 
     // If we have an isa from the command-line, use that. Otherwise if the
     // file contains a unique isa, use that.
     let isa = fisa.isa.or(test_file.isa_spec.unique_isa());
 
     if isa.is_none() {
-        return Err(String::from("compilation requires a target isa"));
+        anyhow::bail!("compilation requires a target isa");
     };
 
     for (func, _) in test_file.functions {
-        let mut relocs = PrintRelocs::new(flag_print);
+        let mut relocs = PrintRelocs::new(options.print);
-        let mut traps = PrintTraps::new(flag_print);
+        let mut traps = PrintTraps::new(options.print);
-        let mut stack_maps = PrintStackMaps::new(flag_print);
+        let mut stack_maps = PrintStackMaps::new(options.print);
 
         if let Some(isa) = isa {
             let mut context = Context::new();
@@ -68,13 +80,15 @@ fn handle_module(
             // Compile and encode the result to machine code.
             let code_info = context
                 .compile_and_emit(isa, &mut mem, &mut relocs, &mut traps, &mut stack_maps)
-                .map_err(|err| pretty_error(&context.func, Some(isa), err))?;
+                .map_err(|err| {
+                    anyhow::anyhow!("{}", pretty_error(&context.func, Some(isa), err))
+                })?;
 
-            if flag_print {
+            if options.print {
                 println!("{}", context.func.display(isa));
             }
 
-            if flag_disasm {
+            if options.disasm {
                 print_all(
                     isa,
                     &mem,
@@ -88,7 +102,7 @@ fn handle_module(
         }
     }
 
-    if flag_report_times {
+    if options.report_times {
         print!("{}", timing::take_current());
     }
 

cranelift/src/disasm.rs

@@ -1,3 +1,4 @@
+use anyhow::Result;
 use cfg_if::cfg_if;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::{binemit, ir};
@@ -124,53 +125,52 @@ cfg_if! {
         use capstone::prelude::*;
         use target_lexicon::Architecture;
 
-        fn get_disassembler(isa: &dyn TargetIsa) -> Result<Capstone, String> {
+        fn get_disassembler(isa: &dyn TargetIsa) -> Result<Capstone> {
             let cs = match isa.triple().architecture {
                 Architecture::Riscv32 | Architecture::Riscv64 => {
-                    return Err(String::from("No disassembler for RiscV"))
+                    anyhow::bail!("No disassembler for RiscV");
                 }
                 Architecture::I386 | Architecture::I586 | Architecture::I686 => Capstone::new()
                     .x86()
                     .mode(arch::x86::ArchMode::Mode32)
-                    .build(),
+                    .build()?,
                 Architecture::X86_64 => Capstone::new()
                     .x86()
                     .mode(arch::x86::ArchMode::Mode64)
-                    .build(),
+                    .build()?,
                 Architecture::Arm(arm) => {
                     if arm.is_thumb() {
                         Capstone::new()
                             .arm()
                             .mode(arch::arm::ArchMode::Thumb)
-                            .build()
+                            .build()?
                     } else {
                         Capstone::new()
                             .arm()
                             .mode(arch::arm::ArchMode::Arm)
-                            .build()
+                            .build()?
                     }
                 }
                 Architecture::Aarch64 {..} => {
                     let mut cs = Capstone::new()
                         .arm64()
                         .mode(arch::arm64::ArchMode::Arm)
-                        .build()
+                        .build()?;
-                        .map_err(|err| err.to_string())?;
                     // AArch64 uses inline constants rather than a separate constant pool right now.
                     // Without this option, Capstone will stop disassembling as soon as it sees
                     // an inline constant that is not also a valid instruction. With this option,
                     // Capstone will print a `.byte` directive with the bytes of the inline constant
                     // and continue to the next instruction.
-                    cs.set_skipdata(true).map_err(|err| err.to_string())?;
+                    cs.set_skipdata(true)?;
-                    Ok(cs)
+                    cs
                 }
-                _ => return Err(String::from("Unknown ISA")),
+                _ => anyhow::bail!("Unknown ISA"),
             };
 
-            cs.map_err(|err| err.to_string())
+            Ok(cs)
         }
 
-        pub fn print_disassembly(isa: &dyn TargetIsa, mem: &[u8]) -> Result<(), String> {
+        pub fn print_disassembly(isa: &dyn TargetIsa, mem: &[u8]) -> Result<()> {
             let cs = get_disassembler(isa)?;
 
             println!("\nDisassembly of {} bytes:", mem.len());
@@ -209,7 +209,7 @@ cfg_if! {
             Ok(())
         }
     } else {
-        pub fn print_disassembly(_: &dyn TargetIsa, _: &[u8]) -> Result<(), String> {
+        pub fn print_disassembly(_: &dyn TargetIsa, _: &[u8]) -> Result<()> {
             println!("\nNo disassembly available.");
             Ok(())
         }
@@ -224,7 +224,7 @@ pub fn print_all(
     relocs: &PrintRelocs,
     traps: &PrintTraps,
     stack_maps: &PrintStackMaps,
-) -> Result<(), String> {
+) -> Result<()> {
     print_bytes(&mem);
     print_disassembly(isa, &mem[0..code_size as usize])?;
     print_readonly_data(&mem[code_size as usize..(code_size + rodata_size) as usize]);

cranelift/src/interpret.rs

@@ -7,23 +7,42 @@ use cranelift_reader::{parse_run_command, parse_test, ParseError, ParseOptions};
 use log::debug;
 use std::path::PathBuf;
 use std::{fs, io};
+use structopt::StructOpt;
 use thiserror::Error;
 
+/// Interpret clif code
+#[derive(StructOpt)]
+pub struct Options {
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(required(true), parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+
+    /// Be more verbose
+    #[structopt(short = "v", long = "verbose")]
+    verbose: bool,
+}
+
 /// Run files through the Cranelift interpreter, interpreting any functions with annotations.
-pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
+pub fn run(options: &Options) -> anyhow::Result<()> {
+    crate::handle_debug_flag(options.debug);
+
     let mut total = 0;
     let mut errors = 0;
-    for file in iterate_files(files) {
+    for file in iterate_files(&options.files) {
         total += 1;
-        let runner = FileInterpreter::from_path(file).map_err(|e| e.to_string())?;
+        let runner = FileInterpreter::from_path(file)?;
         match runner.run() {
             Ok(_) => {
-                if flag_print {
+                if options.verbose {
                     println!("{}", runner.path());
                 }
             }
             Err(e) => {
-                if flag_print {
+                if options.verbose {
                     println!("{}: {}", runner.path(), e.to_string());
                 }
                 errors += 1;
@@ -31,7 +50,7 @@ pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
         }
     }
 
-    if flag_print {
+    if options.verbose {
         match total {
             0 => println!("0 files"),
             1 => println!("1 file"),
@@ -41,8 +60,8 @@ pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
 
     match errors {
         0 => Ok(()),
-        1 => Err(String::from("1 failure")),
+        1 => anyhow::bail!("1 failure"),
-        n => Err(format!("{} failures", n)),
+        n => anyhow::bail!("{} failures", n),
     }
 }
 
@@ -158,6 +177,11 @@ mod test {
 
     #[test]
     fn filetests() {
-        run(vec!["../filetests/filetests/interpreter".to_string()], true).unwrap()
+        run(&Options {
+            files: vec![PathBuf::from("../filetests/filetests/interpreter")],
+            debug: true,
+            verbose: true,
+        })
+        .unwrap()
     }
 }

cranelift/src/print_cfg.rs

@@ -4,12 +4,27 @@
 //! in graphviz format.
 
 use crate::utils::read_to_string;
-use crate::CommandResult;
+use anyhow::Result;
 use cranelift_codegen::cfg_printer::CFGPrinter;
 use cranelift_reader::parse_functions;
+use std::path::{Path, PathBuf};
+use structopt::StructOpt;
 
-pub fn run(files: &[String]) -> CommandResult {
+/// Prints out cfg in GraphViz Dot format
-    for (i, f) in files.iter().enumerate() {
+#[derive(StructOpt)]
+pub struct Options {
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(required(true), parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    crate::handle_debug_flag(options.debug);
+    for (i, f) in options.files.iter().enumerate() {
         if i != 0 {
             println!();
         }
@@ -18,9 +33,9 @@ pub fn run(files: &[String]) -> CommandResult {
     Ok(())
 }
 
-fn print_cfg(filename: &str) -> CommandResult {
+fn print_cfg(path: &Path) -> Result<()> {
-    let buffer = read_to_string(filename).map_err(|e| format!("{}: {}", filename, e))?;
+    let buffer = read_to_string(path)?;
-    let items = parse_functions(&buffer).map_err(|e| format!("{}: {}", filename, e))?;
+    let items = parse_functions(&buffer)?;
 
     for (idx, func) in items.into_iter().enumerate() {
         if idx != 0 {

cranelift/src/run.rs

@@ -1,36 +1,60 @@
 //! CLI tool to compile Cranelift IR files to native code in memory and execute them.
 
 use crate::utils::{iterate_files, read_to_string};
+use anyhow::Result;
 use cranelift_codegen::isa::{CallConv, TargetIsa};
 use cranelift_filetests::SingleFunctionCompiler;
 use cranelift_native::builder as host_isa_builder;
 use cranelift_reader::{parse_run_command, parse_test, Details, IsaSpec, ParseOptions};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
+use structopt::StructOpt;
 use target_lexicon::Triple;
 
-pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
+/// Execute clif code and verify with test expressions
-    let stdin_exist = files.iter().find(|file| *file == "-").is_some();
+#[derive(StructOpt)]
-    let filtered_files = files
+pub struct Options {
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(required(true), parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+
+    /// Be more verbose
+    #[structopt(short = "v", long = "verbose")]
+    verbose: bool,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    crate::handle_debug_flag(options.debug);
+    let stdin_exist = options
+        .files
         .iter()
-        .filter(|file| *file != "-")
+        .find(|file| *file == Path::new("-"))
-        .map(|file| file.to_string())
+        .is_some();
-        .collect::<Vec<String>>();
+    let filtered_files = options
+        .files
+        .iter()
+        .cloned()
+        .filter(|file| *file != Path::new("-"))
+        .collect::<Vec<_>>();
     let mut total = 0;
     let mut errors = 0;
     let mut special_files: Vec<PathBuf> = vec![];
     if stdin_exist {
         special_files.push("-".into());
     }
-    for file in iterate_files(filtered_files).chain(special_files) {
+    for file in iterate_files(&filtered_files).chain(special_files) {
         total += 1;
         match run_single_file(&file) {
             Ok(_) => {
-                if flag_print {
+                if options.verbose {
                     println!("{}", file.to_string_lossy());
                 }
             }
             Err(e) => {
-                if flag_print {
+                if options.verbose {
                     println!("{}: {}", file.to_string_lossy(), e);
                 }
                 errors += 1;
@@ -38,7 +62,7 @@ pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
         }
     }
 
-    if flag_print {
+    if options.verbose {
         match total {
             0 => println!("0 files"),
             1 => println!("1 file"),
@@ -48,33 +72,33 @@ pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
 
     match errors {
         0 => Ok(()),
-        1 => Err(String::from("1 failure")),
+        1 => anyhow::bail!("1 failure"),
-        n => Err(format!("{} failures", n)),
+        n => anyhow::bail!("{} failures", n),
     }
 }
 
 /// Run all functions in a file that are succeeded by "run:" comments
-fn run_single_file(path: &PathBuf) -> Result<(), String> {
+fn run_single_file(path: &PathBuf) -> Result<()> {
-    let file_contents = read_to_string(&path).map_err(|e| e.to_string())?;
+    let file_contents = read_to_string(&path)?;
     run_file_contents(file_contents)
 }
 
 /// Main body of `run_single_file` separated for testing
-fn run_file_contents(file_contents: String) -> Result<(), String> {
+fn run_file_contents(file_contents: String) -> Result<()> {
     let options = ParseOptions {
         default_calling_convention: CallConv::triple_default(&Triple::host()), // use the host's default calling convention
         ..ParseOptions::default()
     };
-    let test_file = parse_test(&file_contents, options).map_err(|e| e.to_string())?;
+    let test_file = parse_test(&file_contents, options)?;
     let isa = create_target_isa(&test_file.isa_spec)?;
     let mut compiler = SingleFunctionCompiler::new(isa);
     for (func, Details { comments, .. }) in test_file.functions {
         for comment in comments {
-            if let Some(command) =
+            if let Some(command) = parse_run_command(comment.text, &func.signature)? {
-                parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())?
+                let compiled_fn = compiler.compile(func.clone())?;
-            {
+                command
-                let compiled_fn = compiler.compile(func.clone()).map_err(|e| e.to_string())?;
+                    .run(|_, args| Ok(compiled_fn.call(args)))
-                command.run(|_, args| Ok(compiled_fn.call(args)))?;
+                    .map_err(|s| anyhow::anyhow!("{}", s))?;
             }
         }
     }
@@ -82,13 +106,16 @@ fn run_file_contents(file_contents: String) -> Result<(), String> {
 }
 
 /// Build an ISA based on the current machine running this code (the host)
-fn create_target_isa(isa_spec: &IsaSpec) -> Result<Box<dyn TargetIsa>, String> {
+fn create_target_isa(isa_spec: &IsaSpec) -> Result<Box<dyn TargetIsa>> {
     if let IsaSpec::None(flags) = isa_spec {
         // build an ISA for the current machine
-        let builder = host_isa_builder()?;
+        let builder = host_isa_builder().map_err(|s| anyhow::anyhow!("{}", s))?;
         Ok(builder.finish(flags.clone()))
     } else {
-        Err(String::from("A target ISA was specified in the file but should not have been--only the host ISA can be used for running CLIF files"))
+        anyhow::bail!(
+            "A target ISA was specified in the file but should not have been--only \
+             the host ISA can be used for running CLIF files"
+        )
     }
 }
 

cranelift/src/souper_harvest.rs

@@ -1,37 +1,65 @@
 use crate::utils::parse_sets_and_triple;
+use anyhow::{Context as _, Result};
 use cranelift_codegen::Context;
 use cranelift_wasm::{DummyEnvironment, ReturnMode};
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
+use std::path::{Path, PathBuf};
 use std::{fs, io};
+use structopt::StructOpt;
 
 static WASM_MAGIC: &[u8] = &[0x00, 0x61, 0x73, 0x6D];
 
-pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Result<(), String> {
+/// Harvest candidates for superoptimization from a Wasm or Clif file.
-    let parsed = parse_sets_and_triple(flag_set, target)?;
+///
+/// Candidates are emitted in Souper's text format:
+/// https://github.com/google/souper
+#[derive(StructOpt)]
+pub struct Options {
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    input: PathBuf,
+
+    /// Specify the output file to be used. Use '-' for stdout.
+    #[structopt(short("o"), long("output"), default_value("-"), parse(from_os_str))]
+    output: PathBuf,
+
+    /// Configure Cranelift settings
+    #[structopt(long("set"))]
+    settings: Vec<String>,
+
+    /// Specify the Cranelift target
+    #[structopt(long("target"))]
+    target: String,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    let parsed = parse_sets_and_triple(&options.settings, &options.target)?;
     let fisa = parsed.as_fisa();
     if fisa.isa.is_none() {
-        return Err("`souper-harvest` requires a target isa".into());
+        anyhow::bail!("`souper-harvest` requires a target isa");
     }
 
     let stdin = io::stdin();
-    let mut input: Box<dyn io::BufRead> = match input {
+    let mut input: Box<dyn io::BufRead> = if options.input == Path::new("-") {
-        "-" => Box::new(stdin.lock()),
+        Box::new(stdin.lock())
-        _ => Box::new(io::BufReader::new(
+    } else {
-            fs::File::open(input).map_err(|e| format!("failed to open input file: {}", e))?,
+        Box::new(io::BufReader::new(
-        )),
+            fs::File::open(&options.input).context("failed to open input file")?,
+        ))
     };
 
-    let mut output: Box<dyn io::Write + Send> = match output {
+    let mut output: Box<dyn io::Write + Send> = if options.output == Path::new("-") {
-        "-" => Box::new(io::stdout()),
+        Box::new(io::stdout())
-        _ => Box::new(io::BufWriter::new(
+    } else {
-            fs::File::create(output).map_err(|e| format!("failed to create output file: {}", e))?,
+        Box::new(io::BufWriter::new(
-        )),
+            fs::File::create(&options.output).context("failed to create output file")?,
+        ))
     };
 
     let mut contents = vec![];
     input
         .read_to_end(&mut contents)
-        .map_err(|e| format!("failed to read from input file: {}", e))?;
+        .context("failed to read input file")?;
 
     let funcs = if &contents[..WASM_MAGIC.len()] == WASM_MAGIC {
         let mut dummy_environ = DummyEnvironment::new(
@@ -40,7 +68,7 @@ pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Resu
             false,
         );
         cranelift_wasm::translate_module(&contents, &mut dummy_environ)
-            .map_err(|e| format!("failed to translate Wasm module to clif: {}", e))?;
+            .context("failed to translate Wasm module to clif")?;
         dummy_environ
             .info
             .function_bodies
@@ -48,19 +76,17 @@ pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Resu
             .map(|(_, f)| f.clone())
             .collect()
     } else {
-        let contents = String::from_utf8(contents)
+        let contents = String::from_utf8(contents)?;
-            .map_err(|e| format!("input is not a UTF-8 string: {}", e))?;
+        cranelift_reader::parse_functions(&contents)?
-        cranelift_reader::parse_functions(&contents)
-            .map_err(|e| format!("failed to parse clif: {}", e))?
     };
 
     let (send, recv) = std::sync::mpsc::channel::<String>();
 
-    let writing_thread = std::thread::spawn(move || -> Result<(), String> {
+    let writing_thread = std::thread::spawn(move || -> Result<()> {
         for lhs in recv {
             output
                 .write_all(lhs.as_bytes())
-                .map_err(|e| format!("failed to write to output file: {}", e))?;
+                .context("failed to write to output file")?;
         }
         Ok(())
     });
@@ -73,14 +99,14 @@ pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Resu
 
             ctx.compute_cfg();
             ctx.preopt(fisa.isa.unwrap())
-                .map_err(|e| format!("failed to run preopt: {}", e))?;
+                .context("failed to run preopt")?;
 
             ctx.souper_harvest(send)
-                .map_err(|e| format!("failed to run souper harvester: {}", e))?;
+                .context("failed to run souper harvester")?;
 
             Ok(())
         })
-        .collect::<Result<(), String>>()?;
+        .collect::<Result<()>>()?;
 
     match writing_thread.join() {
         Ok(result) => result?,

cranelift/src/souper_to_peepmatic.rs

@@ -1,31 +1,43 @@
+use anyhow::{Context, Result};
 use std::io::{Read, Write};
-use std::path::Path;
+use std::path::{Path, PathBuf};
+use structopt::StructOpt;
 
-pub fn run(input: &Path, output: &Path) -> Result<(), String> {
+/// Convert Souper optimizations into Peepmatic DSL.
-    let peepmatic_dsl = if input == Path::new("-") {
+#[derive(StructOpt)]
+pub struct Options {
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    input: PathBuf,
+
+    /// Specify the output file to be used. Use '-' for stdout.
+    #[structopt(short("o"), long("output"), default_value("-"), parse(from_os_str))]
+    output: PathBuf,
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    let peepmatic_dsl = if options.input == Path::new("-") {
         let stdin = std::io::stdin();
         let mut stdin = stdin.lock();
         let mut souper_dsl = vec![];
         stdin
             .read_to_end(&mut souper_dsl)
-            .map_err(|e| format!("failed to read from stdin: {}", e))?;
+            .context("failed to read from stdin")?;
-        let souper_dsl =
+        let souper_dsl = String::from_utf8(souper_dsl).context("stdin is not UTF-8: {}")?;
-            String::from_utf8(souper_dsl).map_err(|e| format!("stdin is not UTF-8: {}", e))?;
+        peepmatic_souper::convert_str(&souper_dsl, Some(Path::new("stdin")))?
-        peepmatic_souper::convert_str(&souper_dsl, Some(Path::new("stdin")))
-            .map_err(|e| e.to_string())?
     } else {
-        peepmatic_souper::convert_file(input).map_err(|e| e.to_string())?
+        peepmatic_souper::convert_file(&options.input)?
     };
 
-    if output == Path::new("-") {
+    if options.output == Path::new("-") {
         let stdout = std::io::stdout();
         let mut stdout = stdout.lock();
         stdout
             .write_all(peepmatic_dsl.as_bytes())
-            .map_err(|e| format!("error writing to stdout: {}", e))?;
+            .context("error writing to stdout")?;
     } else {
-        std::fs::write(output, peepmatic_dsl.as_bytes())
+        std::fs::write(&options.output, peepmatic_dsl.as_bytes())
-            .map_err(|e| format!("error writing to {}: {}", output.display(), e))?;
+            .with_context(|| format!("error writing to {}", options.output.display()))?;
     }
 
     Ok(())

cranelift/src/utils.rs

@@ -1,5 +1,6 @@
 //! Utility functions.
 
+use anyhow::Context;
 use cranelift_codegen::isa;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::settings::{self, FlagsOrIsa};
@@ -12,15 +13,19 @@ use target_lexicon::Triple;
 use walkdir::WalkDir;
 
 /// Read an entire file into a string.
-pub fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String> {
+pub fn read_to_string<P: AsRef<Path>>(path: P) -> anyhow::Result<String> {
     let mut buffer = String::new();
-    if path.as_ref() == Path::new("-") {
+    let path = path.as_ref();
+    if path == Path::new("-") {
         let stdin = io::stdin();
         let mut stdin = stdin.lock();
-        stdin.read_to_string(&mut buffer)?;
+        stdin
+            .read_to_string(&mut buffer)
+            .context("failed to read stdin to string")?;
     } else {
         let mut file = File::open(path)?;
-        file.read_to_string(&mut buffer)?;
+        file.read_to_string(&mut buffer)
+            .with_context(|| format!("failed to read {} to string", path.display()))?;
     }
     Ok(buffer)
 }
@@ -45,7 +50,7 @@ impl OwnedFlagsOrIsa {
 pub fn parse_sets_and_triple(
     flag_set: &[String],
     flag_triple: &str,
-) -> Result<OwnedFlagsOrIsa, String> {
+) -> anyhow::Result<OwnedFlagsOrIsa> {
     let mut flag_builder = settings::builder();
 
     // Collect unknown system-wide settings, so we can try to parse them as target specific
@@ -59,7 +64,7 @@ pub fn parse_sets_and_triple(
         Err(ParseOptionError::UnknownFlag { name, .. }) => {
             unknown_settings.push(name);
         }
-        Err(ParseOptionError::Generic(err)) => return Err(err.to_string()),
+        Err(ParseOptionError::Generic(err)) => return Err(err.into()),
         Ok(()) => {}
     }
 
@@ -68,14 +73,14 @@ pub fn parse_sets_and_triple(
     if let Some(triple_name) = words.next() {
         let triple = match Triple::from_str(triple_name) {
             Ok(triple) => triple,
-            Err(parse_error) => return Err(parse_error.to_string()),
+            Err(parse_error) => return Err(parse_error.into()),
         };
 
         let mut isa_builder = isa::lookup(triple).map_err(|err| match err {
             isa::LookupError::SupportDisabled => {
-                format!("support for triple '{}' is disabled", triple_name)
+                anyhow::anyhow!("support for triple '{}' is disabled", triple_name)
             }
-            isa::LookupError::Unsupported => format!(
+            isa::LookupError::Unsupported => anyhow::anyhow!(
                 "support for triple '{}' is not implemented yet",
                 triple_name
             ),
@@ -87,30 +92,27 @@ pub fn parse_sets_and_triple(
             &mut isa_builder,
             Location { line_number: 0 },
         )
-        .map_err(|err| ParseError::from(err).to_string())?;
+        .map_err(ParseError::from)?;
 
         // Apply the ISA-specific settings to `isa_builder`.
         parse_options(words, &mut isa_builder, Location { line_number: 0 })
-            .map_err(|err| ParseError::from(err).to_string())?;
+            .map_err(ParseError::from)?;
 
         Ok(OwnedFlagsOrIsa::Isa(
             isa_builder.finish(settings::Flags::new(flag_builder)),
         ))
     } else {
         if !unknown_settings.is_empty() {
-            return Err(format!(
+            anyhow::bail!("unknown settings: '{}'", unknown_settings.join("', '"));
-                "unknown settings: '{}'",
-                unknown_settings.join("', '")
-            ));
         }
         Ok(OwnedFlagsOrIsa::Flags(settings::Flags::new(flag_builder)))
     }
 }
 
 /// Iterate over all of the files passed as arguments, recursively iterating through directories.
-pub fn iterate_files(files: Vec<String>) -> impl Iterator<Item = PathBuf> {
+pub fn iterate_files<'a>(files: &'a [PathBuf]) -> impl Iterator<Item = PathBuf> + 'a {
     files
-        .into_iter()
+        .iter()
         .flat_map(WalkDir::new)
         .filter(|f| match f {
             Ok(d) => {

cranelift/src/wasm.rs

@@ -9,7 +9,7 @@
 
 use crate::disasm::{print_all, PrintRelocs, PrintStackMaps, PrintTraps};
 use crate::utils::parse_sets_and_triple;
-use crate::UseTerminalColor;
+use anyhow::{Context as _, Result};
 use cranelift_codegen::ir::DisplayFunctionAnnotations;
 use cranelift_codegen::print_errors::{pretty_error, pretty_verifier_error};
 use cranelift_codegen::settings::FlagsOrIsa;
@@ -20,6 +20,7 @@ use cranelift_wasm::{translate_module, DummyEnvironment, FuncIndex, ReturnMode};
 use std::io::Read;
 use std::path::Path;
 use std::path::PathBuf;
+use structopt::StructOpt;
 use term;
 
 /// For verbose printing: only print if the `$x` expression is true.
@@ -59,69 +60,108 @@ macro_rules! vcprint {
     };
 }
 
-pub fn run(
+/// Compiles Wasm binary/text into Cranelift IR and then into target language
-    files: Vec<String>,
+#[derive(StructOpt)]
-    use_terminal_color: UseTerminalColor,
+pub struct Options {
-    flag_verbose: bool,
+    /// Be more verbose
-    flag_just_decode: bool,
+    #[structopt(short = "v", long = "verbose")]
-    flag_check_translation: bool,
+    verbose: bool,
-    flag_print: bool,
+
-    flag_print_disasm: bool,
+    /// Print the resulting Cranelift IR
-    flag_set: &[String],
+    #[structopt(short("p"))]
-    flag_triple: &str,
+    print: bool,
-    flag_print_size: bool,
+
-    flag_report_times: bool,
+    /// Print pass timing report
-    flag_calc_value_ranges: bool,
+    #[structopt(short("T"))]
-) -> Result<(), String> {
+    report_times: bool,
-    let parsed = parse_sets_and_triple(flag_set, flag_triple)?;
+
-    for filename in files {
+    /// Print machine code disassembly
-        let path = Path::new(&filename);
+    #[structopt(short("D"), long("disasm"))]
+    disasm: bool,
+
+    /// Configure Cranelift settings
+    #[structopt(long("set"))]
+    settings: Vec<String>,
+
+    /// Specify the Cranelift target
+    #[structopt(long("target"))]
+    target: String,
+
+    /// Specify an input file to be used. Use '-' for stdin.
+    #[structopt(parse(from_os_str))]
+    files: Vec<PathBuf>,
+
+    /// Enable debug output on stderr/stdout
+    #[structopt(short = "d")]
+    debug: bool,
+
+    /// Print bytecode size
+    #[structopt(short("X"))]
+    print_size: bool,
+
+    /// Just decode Wasm into Cranelift IR, don't compile it to native code
+    #[structopt(short("t"))]
+    just_decode: bool,
+
+    /// Just checks the correctness of Cranelift IR translated from Wasm
+    #[structopt(short("c"))]
+    check_translation: bool,
+
+    /// Display values' ranges and their locations
+    #[structopt(long("value-ranges"))]
+    value_ranges: bool,
+
+    /// Use colors in output? [options: auto/never/always; default: auto]
+    #[structopt(long("color"), default_value("auto"))]
+    color: Color,
+}
+
+#[derive(PartialEq, Eq)]
+enum Color {
+    Auto,
+    Never,
+    Always,
+}
+
+impl std::str::FromStr for Color {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let s = s.to_lowercase();
+        match s.as_str() {
+            "auto" => Ok(Color::Auto),
+            "never" => Ok(Color::Never),
+            "always" => Ok(Color::Always),
+            _ => Err(format!("expected auto/never/always, found: {}", s)),
+        }
+    }
+}
+
+pub fn run(options: &Options) -> Result<()> {
+    crate::handle_debug_flag(options.debug);
+
+    let parsed = parse_sets_and_triple(&options.settings, &options.target)?;
+    for path in &options.files {
         let name = String::from(path.as_os_str().to_string_lossy());
-        handle_module(
+        handle_module(options, path, &name, parsed.as_fisa())?;
-            use_terminal_color,
-            flag_verbose,
-            flag_just_decode,
-            flag_check_translation,
-            flag_print,
-            flag_print_size,
-            flag_print_disasm,
-            flag_report_times,
-            flag_calc_value_ranges,
-            &path.to_path_buf(),
-            &name,
-            parsed.as_fisa(),
-        )?;
     }
     Ok(())
 }
 
-fn handle_module(
+fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) -> Result<()> {
-    use_terminal_color: UseTerminalColor,
-    flag_verbose: bool,
-    flag_just_decode: bool,
-    flag_check_translation: bool,
-    flag_print: bool,
-    flag_print_size: bool,
-    flag_print_disasm: bool,
-    flag_report_times: bool,
-    flag_calc_value_ranges: bool,
-    path: &PathBuf,
-    name: &str,
-    fisa: FlagsOrIsa,
-) -> Result<(), String> {
     let mut terminal = term::stdout().unwrap();
-    let use_color = terminal.supports_color() && use_terminal_color == UseTerminalColor::Auto
+    let use_color =
-        || use_terminal_color == UseTerminalColor::Always;
+        terminal.supports_color() && options.color == Color::Auto || options.color == Color::Always;
     vcprint!(
-        flag_verbose,
+        options.verbose,
         use_color,
         terminal,
         term::color::YELLOW,
         "Handling: "
     );
-    vprintln!(flag_verbose, "\"{}\"", name);
+    vprintln!(options.verbose, "\"{}\"", name);
     vcprint!(
-        flag_verbose,
+        options.verbose,
         use_color,
         terminal,
         term::color::MAGENTA,
@@ -134,32 +174,34 @@ fn handle_module(
         stdin
             .lock()
             .read_to_end(&mut buf)
-            .map_err(|e| e.to_string())?;
+            .context("failed to read stdin")?;
-        wat::parse_bytes(&buf)
+        wat::parse_bytes(&buf)?.into()
-            .map_err(|err| format!("{:?}", err))?
-            .into()
     } else {
-        wat::parse_file(path).map_err(|err| format!("{:?}", err))?
+        wat::parse_file(path)?
     };
 
     let isa = match fisa.isa {
         Some(isa) => isa,
         None => {
-            return Err(String::from(
+            anyhow::bail!("Error: the wasm command requires an explicit isa.");
-                "Error: the wasm command requires an explicit isa.",
-            ));
         }
     };
 
-    let debug_info = flag_calc_value_ranges;
+    let debug_info = options.value_ranges;
     let mut dummy_environ =
         DummyEnvironment::new(isa.frontend_config(), ReturnMode::NormalReturns, debug_info);
-    translate_module(&module_binary, &mut dummy_environ).map_err(|e| e.to_string())?;
+    translate_module(&module_binary, &mut dummy_environ)?;
 
-    vcprintln!(flag_verbose, use_color, terminal, term::color::GREEN, "ok");
+    vcprintln!(
+        options.verbose,
+        use_color,
+        terminal,
+        term::color::GREEN,
+        "ok"
+    );
 
-    if flag_just_decode {
+    if options.just_decode {
-        if !flag_print {
+        if !options.print {
             return Ok(());
         }
 
@@ -173,22 +215,22 @@ fn handle_module(
                     println!("; Selected as wasm start function");
                 }
             }
-            vprintln!(flag_verbose, "");
+            vprintln!(options.verbose, "");
             for export_name in
                 &dummy_environ.info.functions[FuncIndex::new(func_index)].export_names
             {
                 println!("; Exported as \"{}\"", export_name);
             }
             println!("{}", context.func.display(None));
-            vprintln!(flag_verbose, "");
+            vprintln!(options.verbose, "");
         }
         let _ = terminal.reset();
         return Ok(());
     }
 
-    if flag_check_translation {
+    if options.check_translation {
         vcprint!(
-            flag_verbose,
+            options.verbose,
             use_color,
             terminal,
             term::color::MAGENTA,
@@ -196,7 +238,7 @@ fn handle_module(
         );
     } else {
         vcprint!(
-            flag_verbose,
+            options.verbose,
             use_color,
             terminal,
             term::color::MAGENTA,
@@ -204,8 +246,8 @@ fn handle_module(
         );
     }
 
-    if flag_print_size {
+    if options.print_size {
-        vprintln!(flag_verbose, "");
+        vprintln!(options.verbose, "");
     }
 
     let num_func_imports = dummy_environ.get_num_func_imports();
@@ -217,19 +259,22 @@ fn handle_module(
         let mut saved_sizes = None;
         let func_index = num_func_imports + def_index.index();
         let mut mem = vec![];
-        let mut relocs = PrintRelocs::new(flag_print);
+        let mut relocs = PrintRelocs::new(options.print);
-        let mut traps = PrintTraps::new(flag_print);
+        let mut traps = PrintTraps::new(options.print);
-        let mut stack_maps = PrintStackMaps::new(flag_print);
+        let mut stack_maps = PrintStackMaps::new(options.print);
-        if flag_check_translation {
+        if options.check_translation {
             if let Err(errors) = context.verify(fisa) {
-                return Err(pretty_verifier_error(&context.func, fisa.isa, None, errors));
+                anyhow::bail!(
+                    "{}",
+                    pretty_verifier_error(&context.func, fisa.isa, None, errors)
+                );
             }
         } else {
             let code_info = context
                 .compile_and_emit(isa, &mut mem, &mut relocs, &mut traps, &mut stack_maps)
-                .map_err(|err| pretty_error(&context.func, fisa.isa, err))?;
+                .map_err(|err| anyhow::anyhow!("{}", pretty_error(&context.func, fisa.isa, err)))?;
 
-            if flag_print_size {
+            if options.print_size {
                 println!(
                     "Function #{} code size: {} bytes",
                     func_index, code_info.total_size,
@@ -242,7 +287,7 @@ fn handle_module(
                 );
             }
 
-            if flag_print_disasm {
+            if options.disasm {
                 saved_sizes = Some((
                     code_info.code_size,
                     code_info.jumptables_size + code_info.rodata_size,
@@ -250,8 +295,8 @@ fn handle_module(
             }
         }
 
-        if flag_print {
+        if options.print {
-            vprintln!(flag_verbose, "");
+            vprintln!(options.verbose, "");
             if let Some(start_func) = dummy_environ.info.start_func {
                 if func_index == start_func.index() {
                     println!("; Selected as wasm start function");
@@ -262,7 +307,7 @@ fn handle_module(
             {
                 println!("; Exported as \"{}\"", export_name);
             }
-            let value_ranges = if flag_calc_value_ranges {
+            let value_ranges = if options.value_ranges {
                 Some(
                     context
                         .build_value_labels_ranges(isa)
@@ -278,7 +323,7 @@ fn handle_module(
                     value_ranges: value_ranges.as_ref(),
                 })
             );
-            vprintln!(flag_verbose, "");
+            vprintln!(options.verbose, "");
         }
 
         if let Some((code_size, rodata_size)) = saved_sizes {
@@ -296,16 +341,22 @@ fn handle_module(
         context.clear();
     }
 
-    if !flag_check_translation && flag_print_size {
+    if !options.check_translation && options.print_size {
         println!("Total module code size: {} bytes", total_module_code_size);
         let total_bytecode_size: usize = dummy_environ.func_bytecode_sizes.iter().sum();
         println!("Total module bytecode size: {} bytes", total_bytecode_size);
     }
 
-    if flag_report_times {
+    if options.report_times {
         println!("{}", timing::take_current());
     }
 
-    vcprintln!(flag_verbose, use_color, terminal, term::color::GREEN, "ok");
+    vcprintln!(
+        options.verbose,
+        use_color,
+        terminal,
+        term::color::GREEN,
+        "ok"
+    );
     Ok(())
 }