From 31cbbd1d209f62b6513232b66ecdda66c3e00969 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 14 Sep 2020 16:23:30 -0700 Subject: [PATCH] clif-util: Use `anyhow::Error` for errors instead of `String` Also does the same for `cranelift-filetests`. --- Cargo.lock | 6 +- cranelift/Cargo.toml | 3 +- cranelift/filetests/Cargo.toml | 1 + cranelift/filetests/src/concurrent.rs | 17 +++-- cranelift/filetests/src/lib.rs | 20 ++++-- cranelift/filetests/src/runner.rs | 33 +++++----- cranelift/filetests/src/runone.rs | 39 ++++++----- cranelift/filetests/src/subtest.rs | 19 +++--- cranelift/filetests/src/test_binemit.rs | 50 +++++++-------- cranelift/filetests/src/test_cat.rs | 11 ++-- cranelift/filetests/src/test_compile.rs | 19 ++---- cranelift/filetests/src/test_dce.rs | 14 ++-- cranelift/filetests/src/test_domtree.rs | 44 +++++++------ cranelift/filetests/src/test_interpret.rs | 37 +++++------ cranelift/filetests/src/test_legalizer.rs | 14 ++-- cranelift/filetests/src/test_licm.rs | 14 ++-- cranelift/filetests/src/test_peepmatic.rs | 11 ++-- cranelift/filetests/src/test_postopt.rs | 14 ++-- cranelift/filetests/src/test_preopt.rs | 14 ++-- cranelift/filetests/src/test_print_cfg.rs | 11 ++-- cranelift/filetests/src/test_regalloc.rs | 16 ++--- cranelift/filetests/src/test_rodata.rs | 14 ++-- cranelift/filetests/src/test_run.rs | 23 +++---- cranelift/filetests/src/test_safepoint.rs | 16 ++--- cranelift/filetests/src/test_shrink.rs | 14 ++-- cranelift/filetests/src/test_simple_gvn.rs | 14 ++-- cranelift/filetests/src/test_simple_preopt.rs | 14 ++-- cranelift/filetests/src/test_stack_maps.rs | 14 ++-- cranelift/filetests/src/test_unwind.rs | 11 ++-- cranelift/filetests/src/test_verifier.rs | 17 +++-- cranelift/reader/src/error.rs | 2 + cranelift/src/bugpoint.rs | 34 +++------- cranelift/src/cat.rs | 11 ++-- cranelift/src/clif-util.rs | 64 ++++++++----------- cranelift/src/compile.rs | 17 +++-- cranelift/src/disasm.rs | 30 ++++----- cranelift/src/interpret.rs | 8 +-- cranelift/src/print_cfg.rs | 10 +-- cranelift/src/run.rs | 34 +++++----- cranelift/src/souper_harvest.rs | 29 ++++----- cranelift/src/souper_to_peepmatic.rs | 17 +++-- cranelift/src/utils.rs | 32 +++++----- cranelift/src/wasm.rs | 26 ++++---- 43 files changed, 415 insertions(+), 443 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 38cecde0ee..094bfe93f3 100644 --- a/Cargo.lock +++ b/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,6 +545,7 @@ dependencies = [ name = "cranelift-tools" version = "0.66.0" dependencies = [ + "anyhow", "capstone", "cfg-if", "clap", diff --git a/cranelift/Cargo.toml b/cranelift/Cargo.toml index 5f29620e33..222d282bee 100644 --- a/cranelift/Cargo.toml +++ b/cranelift/Cargo.toml @@ -35,7 +35,7 @@ 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 +43,7 @@ file-per-thread-logger = "0.1.2" indicatif = "0.13.0" thiserror = "1.0.15" walkdir = "2.2" +anyhow = "1.0.32" [features] default = ["disas", "wasm", "cranelift-codegen/all-arch", "peepmatic-souper", "souper-harvest"] diff --git a/cranelift/filetests/Cargo.toml b/cranelift/filetests/Cargo.toml index 296ee65fd9..b4a3afbd08 100644 --- a/cranelift/filetests/Cargo.toml +++ b/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 = [] diff --git a/cranelift/filetests/src/concurrent.rs b/cranelift/filetests/src/concurrent.rs index 67592ed03e..a3d7543f86 100644 --- a/cranelift/filetests/src/concurrent.rs +++ b/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 }, - Done { jobid: usize, result: TestResult }, + Starting { + jobid: usize, + thread_num: usize, + }, + Done { + jobid: usize, + result: anyhow::Result, + }, Tick, } @@ -147,11 +152,11 @@ fn worker_thread( // The test panicked, leaving us a `Box`. // Panics are usually strings. if let Some(msg) = e.downcast_ref::() { - 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) } }); diff --git a/cranelift/filetests/src/lib.rs b/cranelift/filetests/src/lib.rs index 1199086087..affd21e3c4 100644 --- a/cranelift/filetests/src/lib.rs +++ b/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; - /// 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; /// 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 { 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 { 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 ` commands that can appear in /// a `.clif` test file. -fn new_subtest(parsed: &TestCommand) -> subtest::SubtestResult> { +fn new_subtest(parsed: &TestCommand) -> anyhow::Result> { match parsed.command { "binemit" => test_binemit::subtest(parsed), "cat" => test_cat::subtest(parsed), @@ -143,6 +140,15 @@ fn new_subtest(parsed: &TestCommand) -> subtest::SubtestResult 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) +} diff --git a/cranelift/filetests/src/runner.rs b/cranelift/filetests/src/runner.rs index d11ffab79e..e7877cdb4c 100644 --- a/cranelift/filetests/src/runner.rs +++ b/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), } #[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; + let result: anyhow::Result; 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) { - assert_eq!(self.tests[jobid].state, State::Running); + fn finish_job(&mut self, jobid: usize, result: anyhow::Result) { + 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 { 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()), - n => Err(format!("{} failures", n)), + 1 => anyhow::bail!("1 failure"), + 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 { 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()), - n => Err(format!("{} failures", n)), + 1 => anyhow::bail!("1 failure"), + n => anyhow::bail!("{} failures", n), } } } diff --git a/cranelift/filetests/src/runone.rs b/cranelift/filetests/src/runone.rs index 56675a3b0e..14bbb539ce 100644 --- a/cranelift/filetests/src/runone.rs +++ b/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, TestResult}; +use crate::new_subtest; +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 { 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::>>()?; + .collect::>>()?; // 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], isa_spec: &'a IsaSpec, no_isa_flags: &'a Flags, -) -> SubtestResult)>> { +) -> anyhow::Result)>> { 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, 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()) - .map_err(|errors| pretty_verifier_error(&func, isa, None, errors))?; + verify_function(&func, context.flags_or_isa()).map_err(|errors| { + anyhow::anyhow!("{}", pretty_verifier_error(&func, isa, None, errors)) + })?; context.verified = true; } - test.run(func, context) - .map_err(|e| format!("{}:\n{}", name, e)) + test.run(func, context).context(test.name())?; + Ok(()) } diff --git a/cranelift/filetests/src/subtest.rs b/cranelift/filetests/src/subtest.rs index bc65145833..7fd54267d6 100644 --- a/cranelift/filetests/src/subtest.rs +++ b/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 = Result; - /// 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, context: &Context) -> SubtestResult<()>; + fn run(&self, func: Cow, 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))?; - Err(format!("filecheck failed:\n{}{}", checker, explain)) + .context("filecheck explain failed")?; + 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 { +pub fn build_filechecker(context: &Context) -> anyhow::Result { 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()) } diff --git a/cranelift/filetests/src/test_binemit.rs b/cranelift/filetests/src/test_binemit.rs index d5d1860b44..32d8514c92 100644 --- a/cranelift/filetests/src/test_binemit.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { 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, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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::>(); 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!( - "Expected code size {}, got {}", - total_size, sink.offset - )); + anyhow::bail!("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(()) } diff --git a/cranelift/filetests/src/test_cat.rs b/cranelift/filetests/src/test_cat.rs index 756b5e5700..adf16e9403 100644 --- a/cranelift/filetests/src/test_cat.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "cat"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestCat)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestCat)) } impl SubTest for TestCat { @@ -31,7 +30,7 @@ impl SubTest for TestCat { false } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, context: &Context) -> anyhow::Result<()> { subtest::run_filecheck(&func.display(context.isa).to_string(), context) } } diff --git a/cranelift/filetests/src/test_compile.rs b/cranelift/filetests/src/test_compile.rs index a536bd44d8..2a3b5da6b7 100644 --- a/cranelift/filetests/src/test_compile.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "compile"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestCompile)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestCompile)) } impl SubTest for TestCompile { @@ -36,7 +34,7 @@ impl SubTest for TestCompile { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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!( - "Expected code size {}, got {}", - total_size, sink.offset - )); + anyhow::bail!("Expected code size {}, got {}", total_size, sink.offset); } // Run final code through filecheck. diff --git a/cranelift/filetests/src/test_dce.rs b/cranelift/filetests/src/test_dce.rs index b7b72d2a77..826f7cacc9 100644 --- a/cranelift/filetests/src/test_dce.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "dce"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestDCE)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestDCE)) } impl SubTest for TestDCE { @@ -32,14 +30,14 @@ impl SubTest for TestDCE { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_domtree.rs b/cranelift/filetests/src/test_domtree.rs index f5f81ed03a..ba9d51301d 100644 --- a/cranelift/filetests/src/test_domtree.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "domtree"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestDomtree)) + anyhow::bail!("No options allowed on {}", parsed) } + 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, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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 + ); } } diff --git a/cranelift/filetests/src/test_interpret.rs b/cranelift/filetests/src/test_interpret.rs index 1545022375..84398c77d0 100644 --- a/cranelift/filetests/src/test_interpret.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "interpret"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestInterpret)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestInterpret)) } impl SubTest for TestInterpret { @@ -35,26 +34,28 @@ impl SubTest for TestInterpret { false } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, context: &Context) -> anyhow::Result<()> { for comment in context.details.comments.iter() { - if let Some(command) = - parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())? - { + if let Some(command) = parse_run_command(comment.text, &func.signature)? { 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| { - // 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."), - Err(t) => Err(t.to_string()), - } - })?; + 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.") + } + Err(t) => Err(format!("unexpected trap: {:?}", t)), + } + }) + .map_err(|e| anyhow::anyhow!("{}", e))?; } } Ok(()) diff --git a/cranelift/filetests/src/test_legalizer.rs b/cranelift/filetests/src/test_legalizer.rs index ec182f4092..f161226127 100644 --- a/cranelift/filetests/src/test_legalizer.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "legalizer"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestLegalizer)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestLegalizer)) } impl SubTest for TestLegalizer { @@ -34,14 +32,14 @@ impl SubTest for TestLegalizer { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_licm.rs b/cranelift/filetests/src/test_licm.rs index 31385f8081..a6be1d1fe7 100644 --- a/cranelift/filetests/src/test_licm.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "licm"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestLICM)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestLICM)) } impl SubTest for TestLICM { @@ -32,7 +30,7 @@ impl SubTest for TestLICM { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_peepmatic.rs b/cranelift/filetests/src/test_peepmatic.rs index fc701c7046..5d228239a0 100644 --- a/cranelift/filetests/src/test_peepmatic.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { 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, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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); diff --git a/cranelift/filetests/src/test_postopt.rs b/cranelift/filetests/src/test_postopt.rs index 5d5486e912..837257bbc0 100644 --- a/cranelift/filetests/src/test_postopt.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "postopt"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestPostopt)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestPostopt)) } impl SubTest for TestPostopt { @@ -29,14 +27,14 @@ impl SubTest for TestPostopt { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_preopt.rs b/cranelift/filetests/src/test_preopt.rs index 7e0e23475a..072bd7a2ad 100644 --- a/cranelift/filetests/src/test_preopt.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "preopt"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestPreopt)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestPreopt)) } impl SubTest for TestPreopt { @@ -37,12 +35,12 @@ impl SubTest for TestPreopt { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_print_cfg.rs b/cranelift/filetests/src/test_print_cfg.rs index 4483c23939..ff96375134 100644 --- a/cranelift/filetests/src/test_print_cfg.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "print-cfg"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestPrintCfg)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestPrintCfg)) } impl SubTest for TestPrintCfg { @@ -31,7 +30,7 @@ impl SubTest for TestPrintCfg { false } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, context: &Context) -> anyhow::Result<()> { subtest::run_filecheck(&CFGPrinter::new(&func).to_string(), context) } } diff --git a/cranelift/filetests/src/test_regalloc.rs b/cranelift/filetests/src/test_regalloc.rs index 5a316c022f..f0f4025560 100644 --- a/cranelift/filetests/src/test_regalloc.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "regalloc"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestRegalloc)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestRegalloc)) } impl SubTest for TestRegalloc { @@ -36,7 +34,7 @@ impl SubTest for TestRegalloc { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_rodata.rs b/cranelift/filetests/src/test_rodata.rs index 0e6e3cc5f3..6a1976f4fe 100644 --- a/cranelift/filetests/src/test_rodata.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "rodata"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestRodata)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestRodata)) } impl SubTest for TestRodata { @@ -37,13 +35,13 @@ impl SubTest for TestRodata { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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{}", diff --git a/cranelift/filetests/src/test_run.rs b/cranelift/filetests/src/test_run.rs index 05e544efc5..85b0824a7d 100644 --- a/cranelift/filetests/src/test_run.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "run"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestRun)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestRun)) } impl SubTest for TestRun { @@ -35,7 +34,7 @@ impl SubTest for TestRun { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) = - parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())? - { + if let Some(command) = parse_run_command(comment.text, &func.signature)? { 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 - .compile(func.clone().into_owned()) - .map_err(|e| format!("{:?}", e))?; - command.run(|_, args| Ok(compiled_fn.call(args)))?; + let compiled_fn = compiler.compile(func.clone().into_owned())?; + command + .run(|_, args| Ok(compiled_fn.call(args))) + .map_err(|s| anyhow::anyhow!("{}", s))?; } } Ok(()) diff --git a/cranelift/filetests/src/test_safepoint.rs b/cranelift/filetests/src/test_safepoint.rs index b213fb274d..90d155ad1e 100644 --- a/cranelift/filetests/src/test_safepoint.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "safepoint"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestSafepoint)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestSafepoint)) } impl SubTest for TestSafepoint { @@ -20,18 +18,18 @@ impl SubTest for TestSafepoint { "safepoint" } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_shrink.rs b/cranelift/filetests/src/test_shrink.rs index d32da3c384..e3b971b66a 100644 --- a/cranelift/filetests/src/test_shrink.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "shrink"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestShrink)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestShrink)) } impl SubTest for TestShrink { @@ -32,13 +30,13 @@ impl SubTest for TestShrink { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_simple_gvn.rs b/cranelift/filetests/src/test_simple_gvn.rs index 831a632951..66baa0b7a5 100644 --- a/cranelift/filetests/src/test_simple_gvn.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "simple-gvn"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestSimpleGVN)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestSimpleGVN)) } impl SubTest for TestSimpleGVN { @@ -32,13 +30,13 @@ impl SubTest for TestSimpleGVN { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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) diff --git a/cranelift/filetests/src/test_simple_preopt.rs b/cranelift/filetests/src/test_simple_preopt.rs index f6cdec391f..a47d64f5d1 100644 --- a/cranelift/filetests/src/test_simple_preopt.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "simple_preopt"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestSimplePreopt)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestSimplePreopt)) } impl SubTest for TestSimplePreopt { @@ -29,14 +27,14 @@ impl SubTest for TestSimplePreopt { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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); diff --git a/cranelift/filetests/src/test_stack_maps.rs b/cranelift/filetests/src/test_stack_maps.rs index 25c49d7b01..2b70a111d2 100644 --- a/cranelift/filetests/src/test_stack_maps.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "stack_maps"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestStackMaps)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestStackMaps)) } impl SubTest for TestStackMaps { @@ -23,12 +21,12 @@ impl SubTest for TestStackMaps { "stack_maps" } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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( diff --git a/cranelift/filetests/src/test_unwind.rs b/cranelift/filetests/src/test_unwind.rs index faa4ec6cee..65b93bd611 100644 --- a/cranelift/filetests/src/test_unwind.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "unwind"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestUnwind)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestUnwind)) } impl SubTest for TestUnwind { @@ -36,7 +35,7 @@ impl SubTest for TestUnwind { true } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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()); diff --git a/cranelift/filetests/src/test_verifier.rs b/cranelift/filetests/src/test_verifier.rs index 322361e33f..e1d7485365 100644 --- a/cranelift/filetests/src/test_verifier.rs +++ b/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> { +pub fn subtest(parsed: &TestCommand) -> anyhow::Result> { assert_eq!(parsed.command, "verifier"); if !parsed.options.is_empty() { - Err(format!("No options allowed on {}", parsed)) - } else { - Ok(Box::new(TestVerifier)) + anyhow::bail!("No options allowed on {}", parsed); } + Ok(Box::new(TestVerifier)) } impl SubTest for TestVerifier { @@ -38,7 +37,7 @@ impl SubTest for TestVerifier { false } - fn run(&self, func: Cow, context: &Context) -> SubtestResult<()> { + fn run(&self, func: Cow, 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); } } } diff --git a/cranelift/reader/src/error.rs b/cranelift/reader/src/error.rs index bed2352e5c..c8bcb5145f 100644 --- a/cranelift/reader/src/error.rs +++ b/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 = Result; diff --git a/cranelift/src/bugpoint.rs b/cranelift/src/bugpoint.rs index b50fefaab1..81429c5015 100644 --- a/cranelift/src/bugpoint.rs +++ b/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,19 @@ use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::Context; use cranelift_entity::PrimaryMap; use cranelift_reader::{parse_test, ParseOptions}; +use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle}; use std::collections::HashMap; use std::path::Path; -use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle}; - -pub fn run( - filename: &str, - flag_set: &[String], - flag_isa: &str, - verbose: bool, -) -> Result<(), String> { +pub fn run(filename: &str, flag_set: &[String], flag_isa: &str, verbose: bool) -> Result<()> { let parsed = parse_sets_and_triple(flag_set, flag_isa)?; let fisa = parsed.as_fisa(); let path = Path::new(&filename).to_path_buf(); - let buffer = read_to_string(&path).map_err(|e| format!("{}: {}", filename, e))?; - let test_file = - parse_test(&buffer, ParseOptions::default()).map_err(|e| format!("{}: {}", filename, e))?; + let buffer = read_to_string(&path)?; + let test_file = parse_test(&buffer, ParseOptions::default()) + .with_context(|| format!("failed to parse {}", filename))?; // If we have an isa from the command-line, use that. Otherwise if the // file contains a unique isa, use that. @@ -40,7 +35,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 @@ -833,20 +828,11 @@ fn try_resolve_aliases(context: &mut CrashCheckContext, func: &mut Function) { } } -fn reduce( - isa: &dyn TargetIsa, - mut func: Function, - verbose: bool, -) -> Result<(Function, String), String> { +fn reduce(isa: &dyn TargetIsa, mut func: Function, verbose: bool) -> Result<(Function, String)> { let mut context = CrashCheckContext::new(isa); - match context.check_for_crash(&func) { - CheckResult::Succeed => { - return Err( - "Given function compiled successfully or gave a verifier error.".to_string(), - ); - } - CheckResult::Crash(_) => {} + if let CheckResult::Succeed = context.check_for_crash(&func) { + anyhow::bail!("Given function compiled successfully or gave a verifier error."); } try_resolve_aliases(&mut context, &mut func); diff --git a/cranelift/src/cat.rs b/cranelift/src/cat.rs index 4477f10222..247d8b8ea1 100644 --- a/cranelift/src/cat.rs +++ b/cranelift/src/cat.rs @@ -4,10 +4,10 @@ //! normalizing formatting and removing comments. use crate::utils::read_to_string; -use crate::CommandResult; +use anyhow::{Context, Result}; use cranelift_reader::parse_functions; -pub fn run(files: &[String]) -> CommandResult { +pub fn run(files: &[String]) -> Result<()> { for (i, f) in files.iter().enumerate() { if i != 0 { println!(); @@ -17,9 +17,10 @@ pub fn run(files: &[String]) -> CommandResult { Ok(()) } -fn cat_one(filename: &str) -> CommandResult { - let buffer = read_to_string(&filename).map_err(|e| format!("{}: {}", filename, e))?; - let items = parse_functions(&buffer).map_err(|e| format!("{}: {}", filename, e))?; +fn cat_one(filename: &str) -> Result<()> { + let buffer = read_to_string(&filename)?; + let items = + parse_functions(&buffer).with_context(|| format!("failed to parse {}", filename))?; for (idx, func) in items.into_iter().enumerate() { if idx != 0 { diff --git a/cranelift/src/clif-util.rs b/cranelift/src/clif-util.rs index fb07fba621..d3ed23c649 100755 --- a/cranelift/src/clif-util.rs +++ b/cranelift/src/clif-util.rs @@ -16,9 +16,7 @@ use clap::{arg_enum, App, Arg, SubCommand}; use cranelift_codegen::dbg::LOG_FILENAME_PREFIX; use cranelift_codegen::VERSION; -use std::io::{self, Write}; use std::option::Option; -use std::process; mod bugpoint; mod cat; @@ -37,9 +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("-") @@ -199,7 +194,7 @@ fn handle_debug_flag(debug: bool) { } } -fn main() { +fn main() -> anyhow::Result<()> { let app_cmds = App::new("Cranelift code generator utility") .version(VERSION) .subcommand( @@ -276,10 +271,10 @@ fn main() { .arg(add_set_flag()), ); - let res_util = match app_cmds.get_matches().subcommand() { + 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"))) + cat::run(&get_vec(rest_cmd.values_of("file")))? } ("test", Some(rest_cmd)) => { handle_debug_flag(rest_cmd.is_present("debug")); @@ -288,23 +283,21 @@ fn main() { rest_cmd.is_present("time-passes"), &get_vec(rest_cmd.values_of("file")), ) - .map(|_time| ()) + .map_err(|s| anyhow::anyhow!("{}", s))?; } ("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")); @@ -322,11 +315,11 @@ fn main() { target_val, rest_cmd.value_of("single-file").unwrap(), ) - .map(|_time| ()) + .map_err(|s| anyhow::anyhow!("{}", s))?; } ("print-cfg", Some(rest_cmd)) => { handle_debug_flag(rest_cmd.is_present("debug")); - print_cfg::run(&get_vec(rest_cmd.values_of("file"))) + print_cfg::run(&get_vec(rest_cmd.values_of("file")))?; } ("compile", Some(rest_cmd)) => { handle_debug_flag(rest_cmd.is_present("debug")); @@ -343,13 +336,13 @@ fn main() { 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 = { + { let mut target_val: &str = ""; if let Some(clap_target) = rest_cmd.value_of("target") { target_val = clap_target; @@ -368,13 +361,13 @@ fn main() { 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()); - - result + { + anyhow::bail!("Error: clif-util was compiled without wasm support."); + } } ("bugpoint", Some(rest_cmd)) => { let mut target_val: &str = ""; @@ -387,7 +380,7 @@ fn main() { &get_vec(rest_cmd.values_of("set")), target_val, rest_cmd.is_present("verbose"), - ) + )?; } ("souper-to-peepmatic", Some(rest_cmd)) => { #[cfg(feature = "peepmatic-souper")] @@ -396,15 +389,15 @@ fn main() { 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"))] { - Err( - "Error: clif-util was compiled without support for the `souper-to-peepmatic` \ + anyhow::bail!( + "Error: clif-util was compiled without suport for the `souper-to-peepmatic` \ subcommand" .into(), - ) + ); } } ("souper-harvest", Some(rest_cmd)) => { @@ -415,23 +408,16 @@ fn main() { 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"))] { - Err("clif-util was compiled without `souper-harvest` support".into()) + anyhow::bail!("clif-util was compiled without `souper-harvest` support"); } } - _ => Err("Invalid subcommand.".to_owned()), - }; - - if let Err(mut msg) = res_util { - if !msg.ends_with('\n') { - msg.push('\n'); - } - io::stdout().flush().expect("flushing stdout"); - io::stderr().write_all(msg.as_bytes()).unwrap(); - process::exit(1); + _ => anyhow::bail!("Invalid subcommand.".to_owned()), } + + Ok(()) } diff --git a/cranelift/src/compile.rs b/cranelift/src/compile.rs index ceceeaa244..789436f734 100644 --- a/cranelift/src/compile.rs +++ b/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; @@ -17,7 +18,7 @@ pub fn run( flag_report_times: bool, flag_set: &[String], flag_isa: &str, -) -> Result<(), String> { +) -> Result<()> { let parsed = parse_sets_and_triple(flag_set, flag_isa)?; for filename in files { @@ -42,17 +43,17 @@ fn handle_module( 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))?; +) -> Result<()> { + let buffer = read_to_string(&path)?; + let test_file = parse_test(&buffer, ParseOptions::default()) + .with_context(|| format!("failed to parse {}", name))?; // 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 { @@ -68,7 +69,9 @@ 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 { println!("{}", context.func.display(isa)); diff --git a/cranelift/src/disasm.rs b/cranelift/src/disasm.rs index e68f3594a4..c94beab672 100644 --- a/cranelift/src/disasm.rs +++ b/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 { + fn get_disassembler(isa: &dyn TargetIsa) -> Result { 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() - .map_err(|err| err.to_string())?; + .build()?; // 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())?; - Ok(cs) + cs.set_skipdata(true)?; + 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]); diff --git a/cranelift/src/interpret.rs b/cranelift/src/interpret.rs index efd0b22a6d..cf0902645e 100644 --- a/cranelift/src/interpret.rs +++ b/cranelift/src/interpret.rs @@ -10,12 +10,12 @@ use std::{fs, io}; use thiserror::Error; /// Run files through the Cranelift interpreter, interpreting any functions with annotations. -pub fn run(files: Vec, flag_print: bool) -> Result<(), String> { +pub fn run(files: Vec, flag_print: bool) -> anyhow::Result<()> { let mut total = 0; let mut errors = 0; for file in iterate_files(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 { @@ -41,8 +41,8 @@ pub fn run(files: Vec, flag_print: bool) -> Result<(), String> { match errors { 0 => Ok(()), - 1 => Err(String::from("1 failure")), - n => Err(format!("{} failures", n)), + 1 => anyhow::bail!("1 failure"), + n => anyhow::bail!("{} failures", n), } } diff --git a/cranelift/src/print_cfg.rs b/cranelift/src/print_cfg.rs index 2f739f9f9d..aaca3a4e74 100644 --- a/cranelift/src/print_cfg.rs +++ b/cranelift/src/print_cfg.rs @@ -4,11 +4,11 @@ //! 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; -pub fn run(files: &[String]) -> CommandResult { +pub fn run(files: &[String]) -> Result<()> { for (i, f) in files.iter().enumerate() { if i != 0 { println!(); @@ -18,9 +18,9 @@ pub fn run(files: &[String]) -> CommandResult { Ok(()) } -fn print_cfg(filename: &str) -> CommandResult { - let buffer = read_to_string(filename).map_err(|e| format!("{}: {}", filename, e))?; - let items = parse_functions(&buffer).map_err(|e| format!("{}: {}", filename, e))?; +fn print_cfg(filename: &str) -> Result<()> { + let buffer = read_to_string(filename)?; + let items = parse_functions(&buffer)?; for (idx, func) in items.into_iter().enumerate() { if idx != 0 { diff --git a/cranelift/src/run.rs b/cranelift/src/run.rs index e54d56ed2b..a4777d81d0 100644 --- a/cranelift/src/run.rs +++ b/cranelift/src/run.rs @@ -1,6 +1,7 @@ //! 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; @@ -8,7 +9,7 @@ use cranelift_reader::{parse_run_command, parse_test, Details, IsaSpec, ParseOpt use std::path::PathBuf; use target_lexicon::Triple; -pub fn run(files: Vec, flag_print: bool) -> Result<(), String> { +pub fn run(files: Vec, flag_print: bool) -> Result<()> { let stdin_exist = files.iter().find(|file| *file == "-").is_some(); let filtered_files = files .iter() @@ -48,33 +49,33 @@ pub fn run(files: Vec, flag_print: bool) -> Result<(), String> { match errors { 0 => Ok(()), - 1 => Err(String::from("1 failure")), - n => Err(format!("{} failures", n)), + 1 => anyhow::bail!("1 failure"), + 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> { - let file_contents = read_to_string(&path).map_err(|e| e.to_string())?; +fn run_single_file(path: &PathBuf) -> Result<()> { + 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) = - parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())? - { - let compiled_fn = compiler.compile(func.clone()).map_err(|e| e.to_string())?; - command.run(|_, args| Ok(compiled_fn.call(args)))?; + if let Some(command) = parse_run_command(comment.text, &func.signature)? { + let compiled_fn = compiler.compile(func.clone())?; + command + .run(|_, args| Ok(compiled_fn.call(args))) + .map_err(|s| anyhow::anyhow!("{}", s))?; } } } @@ -82,13 +83,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, String> { +fn create_target_isa(isa_spec: &IsaSpec) -> Result> { 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" + ) } } diff --git a/cranelift/src/souper_harvest.rs b/cranelift/src/souper_harvest.rs index 6c15547ae1..2db6881b31 100644 --- a/cranelift/src/souper_harvest.rs +++ b/cranelift/src/souper_harvest.rs @@ -1,4 +1,5 @@ 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}; @@ -6,32 +7,32 @@ use std::{fs, io}; static WASM_MAGIC: &[u8] = &[0x00, 0x61, 0x73, 0x6D]; -pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Result<(), String> { +pub fn run(target: &str, input: &str, output: &str, flag_set: &[String]) -> Result<()> { let parsed = parse_sets_and_triple(flag_set, 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 = match input { "-" => Box::new(stdin.lock()), _ => Box::new(io::BufReader::new( - fs::File::open(input).map_err(|e| format!("failed to open input file: {}", e))?, + fs::File::open(input).context("failed to open input file")?, )), }; let mut output: Box = match output { "-" => Box::new(io::stdout()), _ => Box::new(io::BufWriter::new( - fs::File::create(output).map_err(|e| format!("failed to create output file: {}", e))?, + fs::File::create(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 +41,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 +49,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) - .map_err(|e| format!("input is not a UTF-8 string: {}", e))?; - cranelift_reader::parse_functions(&contents) - .map_err(|e| format!("failed to parse clif: {}", e))? + let contents = String::from_utf8(contents)?; + cranelift_reader::parse_functions(&contents)? }; let (send, recv) = std::sync::mpsc::channel::(); - 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 +72,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::>()?; + .collect::>()?; match writing_thread.join() { Ok(result) => result?, diff --git a/cranelift/src/souper_to_peepmatic.rs b/cranelift/src/souper_to_peepmatic.rs index 755b34c8d5..80cfe2c425 100644 --- a/cranelift/src/souper_to_peepmatic.rs +++ b/cranelift/src/souper_to_peepmatic.rs @@ -1,20 +1,19 @@ +use anyhow::{Context, Result}; use std::io::{Read, Write}; use std::path::Path; -pub fn run(input: &Path, output: &Path) -> Result<(), String> { +pub fn run(input: &Path, output: &Path) -> Result<()> { let peepmatic_dsl = if 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))?; - let souper_dsl = - 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"))) - .map_err(|e| e.to_string())? + .context("failed to read from stdin")?; + let souper_dsl = String::from_utf8(souper_dsl).context("stdin is not UTF-8: {}")?; + peepmatic_souper::convert_str(&souper_dsl, Some(Path::new("stdin")))? } else { - peepmatic_souper::convert_file(input).map_err(|e| e.to_string())? + peepmatic_souper::convert_file(input)? }; if output == Path::new("-") { @@ -22,10 +21,10 @@ pub fn run(input: &Path, output: &Path) -> Result<(), String> { 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()) - .map_err(|e| format!("error writing to {}: {}", output.display(), e))?; + .with_context(|| format!("error writing to {}", output.display()))?; } Ok(()) diff --git a/cranelift/src/utils.rs b/cranelift/src/utils.rs index 7771949536..3e0e11f894 100644 --- a/cranelift/src/utils.rs +++ b/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>(path: P) -> io::Result { +pub fn read_to_string>(path: P) -> anyhow::Result { 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 { +) -> anyhow::Result { 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,21 +92,18 @@ 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!( - "unknown settings: '{}'", - unknown_settings.join("', '") - )); + anyhow::bail!("unknown settings: '{}'", unknown_settings.join("', '")); } Ok(OwnedFlagsOrIsa::Flags(settings::Flags::new(flag_builder))) } diff --git a/cranelift/src/wasm.rs b/cranelift/src/wasm.rs index b98adcbf4a..2e94233b85 100644 --- a/cranelift/src/wasm.rs +++ b/cranelift/src/wasm.rs @@ -10,6 +10,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; @@ -72,7 +73,7 @@ pub fn run( flag_print_size: bool, flag_report_times: bool, flag_calc_value_ranges: bool, -) -> Result<(), String> { +) -> Result<()> { let parsed = parse_sets_and_triple(flag_set, flag_triple)?; for filename in files { let path = Path::new(&filename); @@ -108,7 +109,7 @@ fn handle_module( path: &PathBuf, name: &str, fisa: FlagsOrIsa, -) -> Result<(), String> { +) -> Result<()> { let mut terminal = term::stdout().unwrap(); let use_color = terminal.supports_color() && use_terminal_color == UseTerminalColor::Auto || use_terminal_color == UseTerminalColor::Always; @@ -134,27 +135,23 @@ fn handle_module( stdin .lock() .read_to_end(&mut buf) - .map_err(|e| e.to_string())?; - wat::parse_bytes(&buf) - .map_err(|err| format!("{:?}", err))? - .into() + .context("failed to read stdin")?; + wat::parse_bytes(&buf)?.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( - "Error: the wasm command requires an explicit isa.", - )); + anyhow::bail!("Error: the wasm command requires an explicit isa."); } }; let debug_info = flag_calc_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"); @@ -222,12 +219,15 @@ fn handle_module( let mut stack_maps = PrintStackMaps::new(flag_print); if flag_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 { println!(