cranelift-isle: Rewrite error reporting (#5318)

There were several issues with ISLE's existing error reporting
implementation.

- When using Miette for more readable error reports, it would panic if
  errors were reported from multiple files in the same run.
- Miette is pretty heavy-weight for what we're doing, with a lot of
  dependencies.
- The `Error::Errors` enum variant led to normalization steps in many
  places, to avoid using that variant to represent a single error.

This commit:
- replaces Miette with codespan-reporting
- gets rid of a bunch of cargo-vet exemptions
- replaces the `Error::Errors` variant with a new `Errors` type
- removes source info from `Error` variants so they're easy to construct
- adds source info only when formatting `Errors`
- formats `Errors` with a custom `Debug` impl
- shares common code between ISLE's callers, islec and cranelift-codegen
- includes a source snippet even with fancy-errors disabled

I tried to make this a series of smaller commits but I couldn't find any
good split points; everything was too entangled with everything else.

Author: jameysharp

Cargo.lock

@@ -40,7 +40,6 @@ criterion = "0.3"
 [build-dependencies]
 cranelift-codegen-meta = { path = "meta", version = "0.91.0" }
 cranelift-isle = { path = "../isle/isle", version = "=0.91.0" }
-miette = { version = "5.1.0", features = ["fancy"], optional = true }
 
 [features]
 default = ["std", "unwind"]
@@ -101,7 +100,7 @@ incremental-cache = [
 souper-harvest = ["souper-ir", "souper-ir/stringify"]
 
 # Provide fancy Miette-produced errors for ISLE.
-isle-errors = ["miette", "cranelift-isle/miette-errors"]
+isle-errors = ["cranelift-isle/fancy-errors"]
 
 # Put ISLE generated files in isle_generated_code/, for easier
 # inspection, rather than inside of target/.

cranelift/codegen/Cargo.toml

@@ -15,6 +15,7 @@
 // current directory is used to find the sources.
 
 use cranelift_codegen_meta as meta;
+use cranelift_isle::error::Errors;
 
 use std::env;
 use std::io::Read;
@@ -288,13 +289,16 @@ fn build_isle(
         }
 
         if let Err(e) = run_compilation(compilation) {
-            eprintln!("Error building ISLE files: {:?}", e);
-            let mut source = e.source();
-            while let Some(e) = source {
-                eprintln!("{:?}", e);
-                source = e.source();
-            }
             had_error = true;
+            eprintln!("Error building ISLE files:");
+            eprintln!("{:?}", e);
+            #[cfg(not(feature = "isle-errors"))]
+            {
+                eprintln!("To see a more detailed error report, run: ");
+                eprintln!();
+                eprintln!("    $ cargo check -p cranelift-codegen --features isle-errors");
+                eprintln!();
+            }
         }
     }
 
@@ -311,21 +315,16 @@ fn build_isle(
 ///
 /// NB: This must happen *after* the `cranelift-codegen-meta` functions, since
 /// it consumes files generated by them.
-fn run_compilation(
-    compilation: &IsleCompilation,
-) -> Result<(), Box<dyn std::error::Error + 'static>> {
+fn run_compilation(compilation: &IsleCompilation) -> Result<(), Errors> {
     use cranelift_isle as isle;
 
     eprintln!("Rebuilding {}", compilation.output.display());
 
-    let code = (|| {
-        let lexer = isle::lexer::Lexer::from_files(
-            compilation
-                .inputs
-                .iter()
-                .chain(compilation.untracked_inputs.iter()),
-        )?;
-        let defs = isle::parser::parse(lexer)?;
+    let code = {
+        let file_paths = compilation
+            .inputs
+            .iter()
+            .chain(compilation.untracked_inputs.iter());
 
         let mut options = isle::codegen::CodegenOptions::default();
         // Because we include!() the generated ISLE source, we cannot
@@ -335,62 +334,8 @@ fn run_compilation(
         // https://github.com/rust-lang/rust/issues/47995.)
         options.exclude_global_allow_pragmas = true;
 
-        isle::compile::compile(&defs, &options)
-    })()
-    .map_err(|e| {
-        // Make sure to include the source snippets location info along with
-        // the error messages.
-
-        #[cfg(feature = "isle-errors")]
-        {
-            let report = miette::Report::new(e);
-            return DebugReport(report);
-
-            struct DebugReport(miette::Report);
-
-            impl std::fmt::Display for DebugReport {
-                fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                    self.0.handler().debug(&*self.0, f)
-                }
-            }
-
-            impl std::fmt::Debug for DebugReport {
-                fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                    std::fmt::Display::fmt(self, f)
-                }
-            }
-
-            impl std::error::Error for DebugReport {}
-        }
-        #[cfg(not(feature = "isle-errors"))]
-        {
-            return DebugReport(format!("{}", e));
-
-            struct DebugReport(String);
-
-            impl std::fmt::Display for DebugReport {
-                fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                    writeln!(f, "ISLE errors:\n\n{}\n", self.0)?;
-                    writeln!(f, "To see a more detailed error report, run: ")?;
-                    writeln!(f, "")?;
-                    writeln!(
-                        f,
-                        "    $ cargo check -p cranelift-codegen --features isle-errors"
-                    )?;
-                    writeln!(f, "")?;
-                    Ok(())
-                }
-            }
-
-            impl std::fmt::Debug for DebugReport {
-                fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                    std::fmt::Display::fmt(self, f)
-                }
-            }
-
-            impl std::error::Error for DebugReport {}
-        }
-    })?;
+        isle::compile::from_files(file_paths, &options)?
+    };
 
     let code = rustfmt(&code).unwrap_or_else(|e| {
         println!(
@@ -404,7 +349,8 @@ fn run_compilation(
         "Writing ISLE-generated Rust code to {}",
         compilation.output.display()
     );
-    std::fs::write(&compilation.output, code)?;
+    std::fs::write(&compilation.output, code)
+        .map_err(|e| Errors::from_io(e, "failed writing output"))?;
 
     Ok(())
 }

cranelift/codegen/build.rs

@@ -9,8 +9,8 @@ repository = "https://github.com/bytecodealliance/wasmtime/tree/main/cranelift/i
 version = "0.91.0"
 
 [dependencies]
+codespan-reporting = { version = "0.11.1", optional = true }
 log = { workspace = true, optional = true }
-miette = { version = "5.1.0", optional = true }
 
 [dev-dependencies]
 tempfile = "3"
@@ -19,4 +19,4 @@ tempfile = "3"
 default = []
 
 logging = ["log"]
-miette-errors = ["miette"]
+fancy-errors = ["codespan-reporting"]

cranelift/isle/isle/Cargo.toml

@@ -1,13 +1,25 @@
 //! Compilation process, from AST to Sema to Sequences of Insts.
 
-use crate::error::Result;
+use std::path::Path;
+
+use crate::error::Errors;
 use crate::{ast, codegen, sema, trie};
 
 /// Compile the given AST definitions into Rust source code.
-pub fn compile(defs: &ast::Defs, options: &codegen::CodegenOptions) -> Result<String> {
+pub fn compile(defs: &ast::Defs, options: &codegen::CodegenOptions) -> Result<String, Errors> {
     let mut typeenv = sema::TypeEnv::from_ast(defs)?;
     let termenv = sema::TermEnv::from_ast(&mut typeenv, defs)?;
     crate::overlap::check(&mut typeenv, &termenv)?;
     let tries = trie::build_tries(&termenv);
     Ok(codegen::codegen(&typeenv, &termenv, &tries, options))
 }
+
+/// Compile the given files into Rust source code.
+pub fn from_files<P: AsRef<Path>>(
+    inputs: impl IntoIterator<Item = P>,
+    options: &codegen::CodegenOptions,
+) -> Result<String, Errors> {
+    let lexer = crate::lexer::Lexer::from_files(inputs)?;
+    let defs = crate::parser::parse(lexer)?;
+    compile(&defs, options)
+}