Make less use of thiserror and more unified error handling

crates/rune/src/compile.rs

@@ -8,11 +8,11 @@ pub(crate) use self::assembly::{Assembly, AssemblyInst};
 
 pub(crate) mod attrs;
 
-mod compile_error;
-pub(crate) use self::compile_error::{
+mod error;
+pub(crate) use self::error::{
     CompileErrorKind, HirErrorKind, IrErrorKind, ParseErrorKind, QueryErrorKind, ResolveErrorKind,
 };
-pub use self::compile_error::{Error, ImportStep};
+pub use self::error::{Error, ImportStep};
 
 mod compile_visitor;
 pub use self::compile_visitor::CompileVisitor;

crates/rune/src/compile/compile_error.rs → crates/rune/src/compile/error.rs

@@ -1,3 +1,5 @@
+use core::fmt;
+
 use crate::no_std as std;
 use crate::no_std::io;
 use crate::no_std::path::PathBuf;
@@ -13,14 +15,73 @@ use crate::macros::{SyntheticId, SyntheticKind};
 use crate::parse::{Expectation, Id, IntoExpectation, LexerMode};
 use crate::runtime::debug::DebugSignature;
 use crate::runtime::{AccessError, Label, TypeInfo, TypeOf};
-use crate::shared::ScopeError;
+use crate::shared::scopes::MissingLocal;
 use crate::{Hash, SourceId};
 
-error! {
-    /// An error raised by the compiler.
-    #[derive(Debug)]
-    pub struct Error {
-        kind: CompileErrorKind,
+/// An error raised by the compiler.
+#[derive(Debug)]
+pub struct Error {
+    span: Span,
+    kind: Box<CompileErrorKind>,
+}
+
+impl Error {
+    /// Construct a new compile error.
+    #[allow(unused)]
+    pub(crate) fn new<S, K>(spanned: S, kind: K) -> Self
+    where
+        S: Spanned,
+        CompileErrorKind: From<K>,
+    {
+        Self {
+            span: spanned.span(),
+            kind: Box::new(CompileErrorKind::from(kind)),
+        }
+    }
+
+    /// Construct an error which is made of a single message.
+    pub fn msg<S, M>(spanned: S, message: M) -> Self
+    where
+        S: Spanned,
+        M: fmt::Display,
+    {
+        Self {
+            span: Spanned::span(&spanned),
+            kind: Box::new(CompileErrorKind::Custom {
+                message: message.to_string().into(),
+            }),
+        }
+    }
+
+    /// Get the kind of the error.
+    #[cfg(feature = "emit")]
+    pub(crate) fn kind(&self) -> &CompileErrorKind {
+        &*self.kind
+    }
+
+    /// Convert into the kind of the error.
+    #[cfg(test)]
+    pub(crate) fn into_kind(self) -> CompileErrorKind {
+        *self.kind
+    }
+}
+
+impl Spanned for Error {
+    #[inline]
+    fn span(&self) -> Span {
+        self.span
+    }
+}
+
+impl crate::no_std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn crate::no_std::error::Error + 'static)> {
+        self.kind.source()
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
+        ::core::fmt::Display::fmt(&self.kind, f)
     }
 }
 
@@ -36,6 +97,13 @@ where
     }
 }
 
+impl From<MissingLocal<'_>> for CompileErrorKind {
+    #[inline]
+    fn from(MissingLocal(name): MissingLocal<'_>) -> Self {
+        CompileErrorKind::MissingLocal { name: name.into() }
+    }
+}
+
 impl From<&'static str> for CompileErrorKind {
     fn from(value: &'static str) -> Self {
         CompileErrorKind::Custom {
@@ -132,8 +200,6 @@ pub(crate) enum CompileErrorKind {
     AccessError(#[from] AccessError),
     #[error("{0}")]
     HirError(#[from] HirErrorKind),
-    #[error("{0}")]
-    ScopeError(#[from] ScopeError),
     #[error("Failed to load `{path}`: {error}")]
     FileError {
         path: PathBuf,
@@ -498,12 +564,6 @@ pub(crate) enum IrErrorKind {
         /// The field that was missing.
         field: Box<str>,
     },
-    /// Missing local with the given name.
-    #[error("Missing local `{name}`")]
-    MissingLocal {
-        /// Name of the missing local.
-        name: Box<str>,
-    },
     /// Missing const or local with the given name.
     #[error("No constant or local matching `{name}`")]
     MissingConst {

crates/rune/src/compile/ir.rs

@@ -366,7 +366,7 @@ impl IrPat {
         match self {
             IrPat::Ignore => Ok(true),
             IrPat::Binding(name) => {
-                interp.scopes.decl(name, value, spanned)?;
+                interp.scopes.decl(name, value).with_span(spanned)?;
                 Ok(true)
             }
         }

crates/rune/src/compile/ir/eval.rs

@@ -176,7 +176,7 @@ fn eval_ir_branches(
             None
         };
 
-        interp.scopes.pop(branch, guard)?;
+        interp.scopes.pop(guard).with_span(branch)?;
 
         if let Some(output) = output {
             return Ok(output);
@@ -228,7 +228,7 @@ fn eval_ir_decl(
 ) -> Result<IrValue, IrEvalOutcome> {
     interp.budget.take(ir)?;
     let value = eval_ir(&ir.value, interp, used)?;
-    interp.scopes.decl(&ir.name, value, ir)?;
+    interp.scopes.decl(&ir.name, value).with_span(ir)?;
     Ok(IrValue::Unit)
 }
 
@@ -244,7 +244,7 @@ fn eval_ir_loop(
 
     loop {
         if let Some(condition) = &ir.condition {
-            interp.scopes.clear_current(condition)?;
+            interp.scopes.clear_current().with_span(condition)?;
 
             let value = eval_ir_condition(condition, interp, used)?;
 
@@ -281,7 +281,7 @@ fn eval_ir_loop(
         };
     }
 
-    interp.scopes.pop(ir, guard)?;
+    interp.scopes.pop(guard).with_span(ir)?;
     Ok(IrValue::Unit)
 }
 
@@ -317,7 +317,7 @@ fn eval_ir_scope(
         IrValue::Unit
     };
 
-    interp.scopes.pop(ir, guard)?;
+    interp.scopes.pop(guard).with_span(ir)?;
     Ok(value)
 }
 

crates/rune/src/compile/ir/interpreter.rs

@@ -5,9 +5,10 @@ use crate::compile::{self, IrErrorKind, IrEvalOutcome, IrValue, ItemId, ModId, W
 use crate::compile::{ir, meta};
 use crate::query::{Query, Used};
 use crate::runtime::{ConstValue, Object, Tuple};
+use crate::shared::scopes::MissingLocal;
 
 /// Ir Scopes.
-pub(crate) type IrScopes = crate::shared::Scopes<IrValue>;
+pub(crate) type IrScopes = crate::shared::scopes::Scopes<IrValue>;
 
 /// The interpreter that executed [Ir][crate::ir::Ir].
 pub struct IrInterpreter<'a> {
@@ -129,10 +130,7 @@ impl IrInterpreter<'_> {
         }
 
         if name.starts_with(char::is_lowercase) {
-            Err(compile::Error::new(
-                spanned,
-                IrErrorKind::MissingLocal { name: name.into() },
-            ))
+            Err(compile::Error::new(spanned, MissingLocal(name)))
         } else {
             Err(compile::Error::new(
                 spanned,
@@ -174,17 +172,17 @@ impl IrInterpreter<'_> {
             }
 
             if base.is_empty() {
-                return Err(compile::Error::new(spanned, IrErrorKind::FnNotFound));
+                return Err(compile::Error::new(span, IrErrorKind::FnNotFound));
             }
 
             base.pop();
         };
 
-        let const_fn = self.q.const_fn_for((spanned.span(), id))?;
+        let const_fn = self.q.const_fn_for((span, id))?;
 
         if const_fn.ir_fn.args.len() != args.len() {
             return Err(compile::Error::new(
-                spanned,
+                span,
                 IrErrorKind::ArgumentCountMismatch {
                     actual: args.len(),
                     expected: const_fn.ir_fn.args.len(),
@@ -195,11 +193,11 @@ impl IrInterpreter<'_> {
         let guard = self.scopes.isolate();
 
         for (name, value) in const_fn.ir_fn.args.iter().zip(args) {
-            self.scopes.decl(name, value, spanned)?;
+            self.scopes.decl(name, value).with_span(span)?;
         }
 
         let value = self.eval_value(&const_fn.ir_fn.ir, used)?;
-        self.scopes.pop(spanned, guard)?;
+        self.scopes.pop(guard).with_span(span)?;
         Ok(value)
     }
 }

crates/rune/src/compile/v1.rs

@@ -5,7 +5,7 @@ use crate::compile::ir;
 use crate::compile::meta;
 use crate::compile::{
     self, Assembly, CompileErrorKind, IrBudget, IrCompiler, IrInterpreter, ItemId, ItemMeta,
-    Location, Options,
+    Location, Options, WithSpan,
 };
 use crate::hir;
 use crate::query::{Named, Query, QueryConstFn, Used};
@@ -202,7 +202,7 @@ impl<'a> Assembler<'a> {
 
         for (ir, name) in compiled {
             let value = interpreter.eval_value(&ir, Used::Used)?;
-            interpreter.scopes.decl(name, value, span)?;
+            interpreter.scopes.decl(name, value).with_span(span)?;
         }
 
         interpreter.module = query_const_fn.item_meta.module;

crates/rune/src/compile/v1/assemble.rs

@@ -373,6 +373,7 @@ fn pat_lit_inst(
                 .resolve(resolve_context!(c.q))?
                 .as_i64(true)
                 .with_span(span)?;
+
             return Ok(Some(Inst::EqInteger { integer }));
         }
         hir::ExprKind::Lit(lit) => match lit {
@@ -955,7 +956,8 @@ fn block(hir: &hir::Block<'_>, c: &mut Assembler<'_>, needs: Needs) -> compile::
 
     c.contexts
         .pop()
-        .ok_or_else(|| compile::Error::msg(span, "missing parent context"))?;
+        .ok_or("Missing parent context")
+        .with_span(span)?;
 
     Ok(Asm::top(span))
 }
@@ -1236,7 +1238,9 @@ fn expr_assign(
             let segment = path
                 .first
                 .try_as_ident()
-                .ok_or_else(|| compile::Error::msg(path, "unsupported path"))?;
+                .ok_or("Unsupported path")
+                .with_span(path)?;
+
             let ident = segment.resolve(resolve_context!(c.q))?;
             let var = c.scopes.get_var(c.q.visitor, ident, c.source_id, span)?;
             c.asm.push(Inst::Replace { offset: var.offset }, span);
@@ -1654,7 +1658,8 @@ fn expr_break(
         .scopes
         .total_var_count(span)?
         .checked_sub(last_loop.break_var_count)
-        .ok_or_else(|| compile::Error::msg(span, "var count should be larger"))?;
+        .ok_or("Var count should be larger")
+        .with_span(span)?;
 
     if last_loop.needs.value() {
         if has_value {
@@ -2097,7 +2102,8 @@ fn expr_continue(
         .scopes
         .total_var_count(span)?
         .checked_sub(last_loop.continue_var_count)
-        .ok_or_else(|| compile::Error::msg(span, "var count should be larger"))?;
+        .ok_or("Var count should be larger")
+        .with_span(span)?;
 
     c.locals_pop(vars, span);
 
@@ -2998,7 +3004,8 @@ fn expr_select(
 
     c.contexts
         .pop()
-        .ok_or_else(|| compile::Error::msg(span, "missing parent context"))?;
+        .ok_or("Missing parent context")
+        .with_span(span)?;
 
     Ok(Asm::top(span))
 }

crates/rune/src/diagnostics/fatal.rs

@@ -66,19 +66,19 @@ impl crate::no_std::error::Error for FatalDiagnostic {
 #[allow(missing_docs)]
 #[non_exhaustive]
 pub enum FatalDiagnosticKind {
-    #[error("compile error")]
+    #[error("Compile error")]
     CompileError(
         #[from]
         #[source]
         compile::Error,
     ),
-    #[error("linker error")]
+    #[error("Linker error")]
     LinkError(
         #[from]
         #[source]
         LinkerError,
     ),
     /// An internal error.
-    #[error("internal error: {0}")]
+    #[error("Internal error: {0}")]
     Internal(&'static str),
 }