From 1505b6a9b4965f1d0b352b2a390821aca2ed4743 Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Mon, 21 Feb 2022 19:14:06 +0100
Subject: [PATCH] internal: Split unresolve proc-macro error out of mbe

---
 crates/hir_def/src/body.rs                    |  6 +--
 crates/hir_def/src/lib.rs                     | 12 ++---
 crates/hir_expand/src/builtin_attr_macro.rs   |  5 ++-
 crates/hir_expand/src/builtin_derive_macro.rs | 26 ++++++-----
 crates/hir_expand/src/builtin_fn_macro.rs     | 39 ++++++++--------
 crates/hir_expand/src/db.rs                   | 40 +++++++++++------
 crates/hir_expand/src/eager.rs                | 25 +++++------
 crates/hir_expand/src/lib.rs                  | 29 +++++++++++-
 crates/hir_expand/src/proc_macro.rs           | 15 ++++---
 crates/mbe/src/expander/matcher.rs            |  4 +-
 crates/mbe/src/expander/transcriber.rs        |  4 +-
 crates/mbe/src/lib.rs                         | 44 +++++++++----------
 crates/mbe/src/tt_iter.rs                     |  2 +-
 13 files changed, 147 insertions(+), 104 deletions(-)

diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs
index 8488b4f0d0..a2f64eda06 100644
--- a/crates/hir_def/src/body.rs
+++ b/crates/hir_def/src/body.rs
@@ -97,9 +97,9 @@ impl Expander {
     ) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
         if self.recursion_limit(db).check(self.recursion_limit + 1).is_err() {
             cov_mark::hit!(your_stack_belongs_to_me);
-            return Ok(ExpandResult::str_err(
+            return Ok(ExpandResult::only_err(ExpandError::Other(
                 "reached recursion limit during macro expansion".into(),
-            ));
+            )));
         }
 
         let macro_call = InFile::new(self.current_file_id, &macro_call);
@@ -151,7 +151,7 @@ impl Expander {
                 }
 
                 return ExpandResult::only_err(err.unwrap_or_else(|| {
-                    mbe::ExpandError::Other("failed to parse macro invocation".into())
+                    ExpandError::Other("failed to parse macro invocation".into())
                 }));
             }
         };
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index 452a3712bc..7e33e53599 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -63,8 +63,8 @@ use hir_expand::{
     ast_id_map::FileAstId,
     eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
     hygiene::Hygiene,
-    AstId, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
-    UnresolvedMacro,
+    AstId, ExpandError, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId,
+    MacroDefKind, UnresolvedMacro,
 };
 use item_tree::ExternBlock;
 use la_arena::Idx;
@@ -662,7 +662,7 @@ pub trait AsMacroCall {
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
-        error_sink: &mut dyn FnMut(mbe::ExpandError),
+        error_sink: &mut dyn FnMut(ExpandError),
     ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro>;
 }
 
@@ -672,7 +672,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         db: &dyn db::DefDatabase,
         krate: CrateId,
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
-        mut error_sink: &mut dyn FnMut(mbe::ExpandError),
+        mut error_sink: &mut dyn FnMut(ExpandError),
     ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
         let expands_to = hir_expand::ExpandTo::from_call_site(self.value);
         let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
@@ -681,7 +681,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
             self.value.path().and_then(|path| path::ModPath::from_src(db.upcast(), path, &h));
 
         let path = match error_sink
-            .option(path, || mbe::ExpandError::Other("malformed macro invocation".into()))
+            .option(path, || ExpandError::Other("malformed macro invocation".into()))
         {
             Ok(path) => path,
             Err(error) => {
@@ -719,7 +719,7 @@ fn macro_call_as_call_id(
     db: &dyn db::DefDatabase,
     krate: CrateId,
     resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
-    error_sink: &mut dyn FnMut(mbe::ExpandError),
+    error_sink: &mut dyn FnMut(ExpandError),
 ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
     let def: MacroDefId =
         resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
diff --git a/crates/hir_expand/src/builtin_attr_macro.rs b/crates/hir_expand/src/builtin_attr_macro.rs
index 8da8c2ee4f..907ee02e33 100644
--- a/crates/hir_expand/src/builtin_attr_macro.rs
+++ b/crates/hir_expand/src/builtin_attr_macro.rs
@@ -1,9 +1,10 @@
 //! Builtin attributes.
 
-use mbe::ExpandResult;
 use syntax::ast;
 
-use crate::{db::AstDatabase, name, AstId, CrateId, MacroCallId, MacroDefId, MacroDefKind};
+use crate::{
+    db::AstDatabase, name, AstId, CrateId, ExpandResult, MacroCallId, MacroDefId, MacroDefKind,
+};
 
 macro_rules! register_builtin {
     ( $(($name:ident, $variant:ident) => $expand:ident),* ) => {
diff --git a/crates/hir_expand/src/builtin_derive_macro.rs b/crates/hir_expand/src/builtin_derive_macro.rs
index bd75c51cbc..dd7d249efa 100644
--- a/crates/hir_expand/src/builtin_derive_macro.rs
+++ b/crates/hir_expand/src/builtin_derive_macro.rs
@@ -2,13 +2,16 @@
 
 use tracing::debug;
 
-use mbe::ExpandResult;
 use syntax::{
     ast::{self, AstNode, HasGenericParams, HasModuleItem, HasName},
     match_ast,
 };
+use tt::TokenId;
 
-use crate::{db::AstDatabase, name, quote, AstId, CrateId, MacroCallId, MacroDefId, MacroDefKind};
+use crate::{
+    db::AstDatabase, name, quote, AstId, CrateId, ExpandError, ExpandResult, MacroCallId,
+    MacroDefId, MacroDefKind,
+};
 
 macro_rules! register_builtin {
     ( $($trait:ident => $expand:ident),* ) => {
@@ -71,15 +74,15 @@ struct BasicAdtInfo {
     type_params: usize,
 }
 
-fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, mbe::ExpandError> {
-    let (parsed, token_map) = mbe::token_tree_to_syntax_node(tt, mbe::TopEntryPoint::MacroItems); // FragmentKind::Items doesn't parse attrs?
+fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, ExpandError> {
+    let (parsed, token_map) = mbe::token_tree_to_syntax_node(tt, mbe::TopEntryPoint::MacroItems);
     let macro_items = ast::MacroItems::cast(parsed.syntax_node()).ok_or_else(|| {
         debug!("derive node didn't parse");
-        mbe::ExpandError::UnexpectedToken
+        ExpandError::Other("invalid item definition".into())
     })?;
     let item = macro_items.items().next().ok_or_else(|| {
         debug!("no module item parsed");
-        mbe::ExpandError::NoMatchingRule
+        ExpandError::Other("no item found".into())
     })?;
     let node = item.syntax();
     let (name, params) = match_ast! {
@@ -89,18 +92,17 @@ fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, mbe::ExpandError> {
             ast::Union(it) => (it.name(), it.generic_param_list()),
             _ => {
                 debug!("unexpected node is {:?}", node);
-                return Err(mbe::ExpandError::ConversionError)
+                return Err(ExpandError::Other("expected struct, enum or union".into()))
             },
         }
     };
     let name = name.ok_or_else(|| {
         debug!("parsed item has no name");
-        mbe::ExpandError::NoMatchingRule
-    })?;
-    let name_token_id = token_map.token_by_range(name.syntax().text_range()).ok_or_else(|| {
-        debug!("name token not found");
-        mbe::ExpandError::ConversionError
+        ExpandError::Other("missing name".into())
     })?;
+    let name_token_id = token_map
+        .token_by_range(name.syntax().text_range())
+        .unwrap_or_else(|| TokenId::unspecified());
     let name_token = tt::Ident { id: name_token_id, text: name.text().into() };
     let type_params = params.map_or(0, |type_param_list| type_param_list.type_params().count());
     Ok(BasicAdtInfo { name: name_token, type_params })
diff --git a/crates/hir_expand/src/builtin_fn_macro.rs b/crates/hir_expand/src/builtin_fn_macro.rs
index 1b49fc0ab4..5876be81b4 100644
--- a/crates/hir_expand/src/builtin_fn_macro.rs
+++ b/crates/hir_expand/src/builtin_fn_macro.rs
@@ -1,15 +1,16 @@
 //! Builtin macro
-use crate::{
-    db::AstDatabase, name, quote, AstId, CrateId, MacroCallId, MacroCallLoc, MacroDefId,
-    MacroDefKind,
-};
 
 use base_db::{AnchoredPath, Edition, FileId};
 use cfg::CfgExpr;
 use either::Either;
-use mbe::{parse_exprs_with_sep, parse_to_token_tree, ExpandResult};
+use mbe::{parse_exprs_with_sep, parse_to_token_tree};
 use syntax::ast::{self, AstToken};
 
+use crate::{
+    db::AstDatabase, name, quote, AstId, CrateId, ExpandError, ExpandResult, MacroCallId,
+    MacroCallLoc, MacroDefId, MacroDefKind,
+};
+
 macro_rules! register_builtin {
     ( LAZY: $(($name:ident, $kind: ident) => $expand:ident),* , EAGER: $(($e_name:ident, $e_kind: ident) => $e_expand:ident),*  ) => {
         #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -257,7 +258,7 @@ fn format_args_expand(
     let mut args = parse_exprs_with_sep(tt, ',');
 
     if args.is_empty() {
-        return ExpandResult::only_err(mbe::ExpandError::NoMatchingRule);
+        return ExpandResult::only_err(mbe::ExpandError::NoMatchingRule.into());
     }
     for arg in &mut args {
         // Remove `key =`.
@@ -368,12 +369,12 @@ fn compile_error_expand(
             let text = it.text.as_str();
             if text.starts_with('"') && text.ends_with('"') {
                 // FIXME: does not handle raw strings
-                mbe::ExpandError::Other(text[1..text.len() - 1].into())
+                ExpandError::Other(text[1..text.len() - 1].into())
             } else {
-                mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into())
+                ExpandError::Other("`compile_error!` argument must be a string".into())
             }
         }
-        _ => mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into()),
+        _ => ExpandError::Other("`compile_error!` argument must be a string".into()),
     };
 
     ExpandResult { value: ExpandedEager::new(quote! {}), err: Some(err) }
@@ -414,7 +415,7 @@ fn concat_expand(
             }
             tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
             _ => {
-                err.get_or_insert(mbe::ExpandError::UnexpectedToken);
+                err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
             }
         }
     }
@@ -435,7 +436,7 @@ fn concat_idents_expand(
             }
             tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
             _ => {
-                err.get_or_insert(mbe::ExpandError::UnexpectedToken);
+                err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
             }
         }
     }
@@ -448,28 +449,28 @@ fn relative_file(
     call_id: MacroCallId,
     path_str: &str,
     allow_recursion: bool,
-) -> Result<FileId, mbe::ExpandError> {
+) -> Result<FileId, ExpandError> {
     let call_site = call_id.as_file().original_file(db);
     let path = AnchoredPath { anchor: call_site, path: path_str };
-    let res = db.resolve_path(path).ok_or_else(|| {
-        mbe::ExpandError::Other(format!("failed to load file `{path_str}`").into())
-    })?;
+    let res = db
+        .resolve_path(path)
+        .ok_or_else(|| ExpandError::Other(format!("failed to load file `{path_str}`").into()))?;
     // Prevent include itself
     if res == call_site && !allow_recursion {
-        Err(mbe::ExpandError::Other(format!("recursive inclusion of `{path_str}`").into()))
+        Err(ExpandError::Other(format!("recursive inclusion of `{path_str}`").into()))
     } else {
         Ok(res)
     }
 }
 
-fn parse_string(tt: &tt::Subtree) -> Result<String, mbe::ExpandError> {
+fn parse_string(tt: &tt::Subtree) -> Result<String, ExpandError> {
     tt.token_trees
         .get(0)
         .and_then(|tt| match tt {
             tt::TokenTree::Leaf(tt::Leaf::Literal(it)) => unquote_str(it),
             _ => None,
         })
-        .ok_or(mbe::ExpandError::ConversionError)
+        .ok_or(mbe::ExpandError::ConversionError.into())
 }
 
 fn include_expand(
@@ -561,7 +562,7 @@ fn env_expand(
         // The only variable rust-analyzer ever sets is `OUT_DIR`, so only diagnose that to avoid
         // unnecessary diagnostics for eg. `CARGO_PKG_NAME`.
         if key == "OUT_DIR" {
-            err = Some(mbe::ExpandError::Other(
+            err = Some(ExpandError::Other(
                 r#"`OUT_DIR` not set, enable "run build scripts" to fix"#.into(),
             ));
         }
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 7d82b33db8..91c1631e81 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -5,7 +5,7 @@ use std::sync::Arc;
 use base_db::{salsa, SourceDatabase};
 use either::Either;
 use limit::Limit;
-use mbe::{syntax_node_to_token_tree, ExpandError, ExpandResult};
+use mbe::syntax_node_to_token_tree;
 use rustc_hash::FxHashSet;
 use syntax::{
     algo::diff,
@@ -15,8 +15,9 @@ use syntax::{
 
 use crate::{
     ast_id_map::AstIdMap, fixup, hygiene::HygieneFrame, BuiltinAttrExpander, BuiltinDeriveExpander,
-    BuiltinFnLikeExpander, ExpandTo, HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind,
-    MacroCallLoc, MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
+    BuiltinFnLikeExpander, ExpandError, ExpandResult, ExpandTo, HirFileId, HirFileIdRepr,
+    MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, MacroFile,
+    ProcMacroExpander,
 };
 
 /// Total limit on the number of tokens produced by any macro invocation.
@@ -47,10 +48,10 @@ impl TokenExpander {
         db: &dyn AstDatabase,
         id: MacroCallId,
         tt: &tt::Subtree,
-    ) -> mbe::ExpandResult<tt::Subtree> {
+    ) -> ExpandResult<tt::Subtree> {
         match self {
-            TokenExpander::DeclarativeMacro { mac, .. } => mac.expand(tt),
-            TokenExpander::Builtin(it) => it.expand(db, id, tt),
+            TokenExpander::DeclarativeMacro { mac, .. } => mac.expand(tt).map_err(Into::into),
+            TokenExpander::Builtin(it) => it.expand(db, id, tt).map_err(Into::into),
             TokenExpander::BuiltinAttr(it) => it.expand(db, id, tt),
             TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt),
             TokenExpander::ProcMacro(_) => {
@@ -432,7 +433,11 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult<Option<Ar
 
     let macro_arg = match db.macro_arg(id) {
         Some(it) => it,
-        None => return ExpandResult::str_err("Failed to lower macro args to token tree".into()),
+        None => {
+            return ExpandResult::only_err(ExpandError::Other(
+                "Failed to lower macro args to token tree".into(),
+            ))
+        }
     };
 
     let expander = match db.macro_def(loc.def) {
@@ -440,16 +445,23 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult<Option<Ar
         // FIXME: This is weird -- we effectively report macro *definition*
         // errors lazily, when we try to expand the macro. Instead, they should
         // be reported at the definition site (when we construct a def map).
-        Err(err) => return ExpandResult::str_err(format!("invalid macro definition: {}", err)),
+        Err(err) => {
+            return ExpandResult::only_err(ExpandError::Other(
+                format!("invalid macro definition: {}", err).into(),
+            ))
+        }
     };
     let ExpandResult { value: mut tt, err } = expander.expand(db, id, &macro_arg.0);
     // Set a hard limit for the expanded tt
     let count = tt.count();
     if TOKEN_LIMIT.check(count).is_err() {
-        return ExpandResult::str_err(format!(
-            "macro invocation exceeds token limit: produced {} tokens, limit is {}",
-            count,
-            TOKEN_LIMIT.inner(),
+        return ExpandResult::only_err(ExpandError::Other(
+            format!(
+                "macro invocation exceeds token limit: produced {} tokens, limit is {}",
+                count,
+                TOKEN_LIMIT.inner(),
+            )
+            .into(),
         ));
     }
 
@@ -466,7 +478,9 @@ fn expand_proc_macro(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult<tt::
     let loc: MacroCallLoc = db.lookup_intern_macro_call(id);
     let macro_arg = match db.macro_arg(id) {
         Some(it) => it,
-        None => return ExpandResult::str_err("No arguments for proc-macro".to_string()),
+        None => {
+            return ExpandResult::only_err(ExpandError::Other("No arguments for proc-macro".into()))
+        }
     };
 
     let expander = match loc.def.kind {
diff --git a/crates/hir_expand/src/eager.rs b/crates/hir_expand/src/eager.rs
index 66f7d8e3bc..1de0d5a77d 100644
--- a/crates/hir_expand/src/eager.rs
+++ b/crates/hir_expand/src/eager.rs
@@ -21,7 +21,6 @@
 use std::sync::Arc;
 
 use base_db::CrateId;
-use mbe::ExpandResult;
 use syntax::{ted, SyntaxNode};
 
 use crate::{
@@ -29,8 +28,8 @@ use crate::{
     db::AstDatabase,
     hygiene::Hygiene,
     mod_path::ModPath,
-    EagerCallInfo, ExpandTo, InFile, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId,
-    MacroDefKind, UnresolvedMacro,
+    EagerCallInfo, ExpandError, ExpandResult, ExpandTo, InFile, MacroCallId, MacroCallKind,
+    MacroCallLoc, MacroDefId, MacroDefKind, UnresolvedMacro,
 };
 
 #[derive(Debug)]
@@ -39,12 +38,12 @@ pub struct ErrorEmitted {
 }
 
 pub trait ErrorSink {
-    fn emit(&mut self, err: mbe::ExpandError);
+    fn emit(&mut self, err: ExpandError);
 
     fn option<T>(
         &mut self,
         opt: Option<T>,
-        error: impl FnOnce() -> mbe::ExpandError,
+        error: impl FnOnce() -> ExpandError,
     ) -> Result<T, ErrorEmitted> {
         match opt {
             Some(it) => Ok(it),
@@ -58,12 +57,12 @@ pub trait ErrorSink {
     fn option_with<T>(
         &mut self,
         opt: impl FnOnce() -> Option<T>,
-        error: impl FnOnce() -> mbe::ExpandError,
+        error: impl FnOnce() -> ExpandError,
     ) -> Result<T, ErrorEmitted> {
         self.option(opt(), error)
     }
 
-    fn result<T>(&mut self, res: Result<T, mbe::ExpandError>) -> Result<T, ErrorEmitted> {
+    fn result<T>(&mut self, res: Result<T, ExpandError>) -> Result<T, ErrorEmitted> {
         match res {
             Ok(it) => Ok(it),
             Err(e) => {
@@ -90,8 +89,8 @@ pub trait ErrorSink {
     }
 }
 
-impl ErrorSink for &'_ mut dyn FnMut(mbe::ExpandError) {
-    fn emit(&mut self, err: mbe::ExpandError) {
+impl ErrorSink for &'_ mut dyn FnMut(ExpandError) {
+    fn emit(&mut self, err: ExpandError) {
         self(err);
     }
 }
@@ -102,7 +101,7 @@ pub fn expand_eager_macro(
     macro_call: InFile<ast::MacroCall>,
     def: MacroDefId,
     resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,
-    diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
+    diagnostic_sink: &mut dyn FnMut(ExpandError),
 ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
     let hygiene = Hygiene::new(db, macro_call.file_id);
     let parsed_args = macro_call
@@ -147,7 +146,7 @@ pub fn expand_eager_macro(
     if let MacroDefKind::BuiltInEager(eager, _) = def.kind {
         let res = eager.expand(db, arg_id, &subtree);
         if let Some(err) = res.err {
-            diagnostic_sink(err);
+            diagnostic_sink(err.into());
         }
 
         let loc = MacroCallLoc {
@@ -199,7 +198,7 @@ fn eager_macro_recur(
     curr: InFile<SyntaxNode>,
     krate: CrateId,
     macro_resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,
-    mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
+    mut diagnostic_sink: &mut dyn FnMut(ExpandError),
 ) -> Result<Result<SyntaxNode, ErrorEmitted>, UnresolvedMacro> {
     let original = curr.value.clone_for_update();
 
@@ -211,7 +210,7 @@ fn eager_macro_recur(
         let def = match child.path().and_then(|path| ModPath::from_src(db, path, &hygiene)) {
             Some(path) => macro_resolver(path.clone()).ok_or_else(|| UnresolvedMacro { path })?,
             None => {
-                diagnostic_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
+                diagnostic_sink(ExpandError::Other("malformed macro invocation".into()));
                 continue;
             }
         };
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index e33c2565c3..27c3f097ab 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -17,9 +17,9 @@ pub mod eager;
 pub mod mod_path;
 mod fixup;
 
-pub use mbe::{ExpandError, ExpandResult, Origin};
+pub use mbe::{Origin, ValueResult};
 
-use std::{hash::Hash, iter, sync::Arc};
+use std::{fmt, hash::Hash, iter, sync::Arc};
 
 use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange, ProcMacroKind};
 use either::Either;
@@ -39,6 +39,31 @@ use crate::{
     proc_macro::ProcMacroExpander,
 };
 
+pub type ExpandResult<T> = ValueResult<T, ExpandError>;
+
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub enum ExpandError {
+    UnresolvedProcMacro,
+    Mbe(mbe::ExpandError),
+    Other(Box<str>),
+}
+
+impl From<mbe::ExpandError> for ExpandError {
+    fn from(mbe: mbe::ExpandError) -> Self {
+        Self::Mbe(mbe)
+    }
+}
+
+impl fmt::Display for ExpandError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ExpandError::UnresolvedProcMacro => f.write_str("unresolved proc-macro"),
+            ExpandError::Mbe(it) => it.fmt(f),
+            ExpandError::Other(it) => f.write_str(it),
+        }
+    }
+}
+
 /// Input to the analyzer is a set of files, where each file is identified by
 /// `FileId` and contains source code. However, another source of source code in
 /// Rust are macros: each macro can be thought of as producing a "temporary
diff --git a/crates/hir_expand/src/proc_macro.rs b/crates/hir_expand/src/proc_macro.rs
index 27c45f002b..df6c38761c 100644
--- a/crates/hir_expand/src/proc_macro.rs
+++ b/crates/hir_expand/src/proc_macro.rs
@@ -1,9 +1,8 @@
 //! Proc Macro Expander stub
 
 use base_db::{CrateId, ProcMacroExpansionError, ProcMacroId, ProcMacroKind};
-use mbe::ExpandResult;
 
-use crate::db::AstDatabase;
+use crate::{db::AstDatabase, ExpandError, ExpandResult};
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
 pub struct ProcMacroExpander {
@@ -37,7 +36,11 @@ impl ProcMacroExpander {
                 let krate_graph = db.crate_graph();
                 let proc_macro = match krate_graph[self.krate].proc_macro.get(id.0 as usize) {
                     Some(proc_macro) => proc_macro,
-                    None => return ExpandResult::str_err("No proc-macro found.".to_string()),
+                    None => {
+                        return ExpandResult::only_err(ExpandError::Other(
+                            "No proc-macro found.".into(),
+                        ))
+                    }
                 };
 
                 // Proc macros have access to the environment variables of the invoking crate.
@@ -51,17 +54,17 @@ impl ProcMacroExpander {
                         {
                             ExpandResult {
                                 value: tt.clone(),
-                                err: Some(mbe::ExpandError::Other(text.into())),
+                                err: Some(ExpandError::Other(text.into())),
                             }
                         }
                         ProcMacroExpansionError::System(text)
                         | ProcMacroExpansionError::Panic(text) => {
-                            ExpandResult::only_err(mbe::ExpandError::Other(text.into()))
+                            ExpandResult::only_err(ExpandError::Other(text.into()))
                         }
                     },
                 }
             }
-            None => ExpandResult::only_err(mbe::ExpandError::UnresolvedProcMacro),
+            None => ExpandResult::only_err(ExpandError::UnresolvedProcMacro),
         }
     }
 }
diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs
index b4c6d3bf61..944d3ef87d 100644
--- a/crates/mbe/src/expander/matcher.rs
+++ b/crates/mbe/src/expander/matcher.rs
@@ -599,7 +599,7 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
                 src = it;
                 res.unmatched_tts += src.len();
             }
-            res.add_err(ExpandError::binding_error("leftover tokens"));
+            res.add_err(ExpandError::LeftoverTokens);
 
             if let Some(error_reover_item) = error_recover_item {
                 res.bindings = bindings_builder.build(&error_reover_item);
@@ -658,7 +658,7 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
 fn match_leaf(lhs: &tt::Leaf, src: &mut TtIter) -> Result<(), ExpandError> {
     let rhs = src
         .expect_leaf()
-        .map_err(|()| ExpandError::BindingError(format!("expected leaf: `{lhs}`").into()))?;
+        .map_err(|()| ExpandError::binding_error(format!("expected leaf: `{lhs}`")))?;
     match (lhs, rhs) {
         (
             tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs
index e24a75a9b0..b1b3f63fd3 100644
--- a/crates/mbe/src/expander/transcriber.rs
+++ b/crates/mbe/src/expander/transcriber.rs
@@ -17,7 +17,7 @@ impl Bindings {
 
     fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> {
         macro_rules! binding_err {
-            ($($arg:tt)*) => { ExpandError::BindingError(format!($($arg)*).into()) };
+            ($($arg:tt)*) => { ExpandError::binding_error(format!($($arg)*)) };
         }
 
         let mut b: &Binding =
@@ -178,7 +178,7 @@ fn expand_repeat(
             );
             return ExpandResult {
                 value: Fragment::Tokens(Subtree::default().into()),
-                err: Some(ExpandError::Other("Expand exceed limit".into())),
+                err: Some(ExpandError::LimitExceeded),
             };
         }
 
diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs
index 07b7f4d1a5..6402ceadaa 100644
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@@ -67,18 +67,17 @@ impl fmt::Display for ParseError {
 
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum ExpandError {
+    BindingError(Box<Box<str>>),
+    LeftoverTokens,
+    ConversionError,
+    LimitExceeded,
     NoMatchingRule,
     UnexpectedToken,
-    BindingError(Box<str>),
-    ConversionError,
-    // FIXME: no way mbe should know about proc macros.
-    UnresolvedProcMacro,
-    Other(Box<str>),
 }
 
 impl ExpandError {
-    fn binding_error(e: &str) -> ExpandError {
-        ExpandError::BindingError(e.into())
+    fn binding_error(e: impl Into<Box<str>>) -> ExpandError {
+        ExpandError::BindingError(Box::new(e.into()))
     }
 }
 
@@ -89,8 +88,8 @@ impl fmt::Display for ExpandError {
             ExpandError::UnexpectedToken => f.write_str("unexpected token in input"),
             ExpandError::BindingError(e) => f.write_str(e),
             ExpandError::ConversionError => f.write_str("could not convert tokens"),
-            ExpandError::UnresolvedProcMacro => f.write_str("unresolved proc macro"),
-            ExpandError::Other(e) => f.write_str(e),
+            ExpandError::LimitExceeded => f.write_str("Expand exceed limit"),
+            ExpandError::LeftoverTokens => f.write_str("leftover tokens"),
         }
     }
 }
@@ -311,42 +310,41 @@ fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> {
     Ok(())
 }
 
+pub type ExpandResult<T> = ValueResult<T, ExpandError>;
+
 #[derive(Debug, Clone, Eq, PartialEq)]
-pub struct ExpandResult<T> {
+pub struct ValueResult<T, E> {
     pub value: T,
-    pub err: Option<ExpandError>,
+    pub err: Option<E>,
 }
 
-impl<T> ExpandResult<T> {
+impl<T, E> ValueResult<T, E> {
     pub fn ok(value: T) -> Self {
         Self { value, err: None }
     }
 
-    pub fn only_err(err: ExpandError) -> Self
+    pub fn only_err(err: E) -> Self
     where
         T: Default,
     {
         Self { value: Default::default(), err: Some(err) }
     }
 
-    pub fn str_err(err: String) -> Self
-    where
-        T: Default,
-    {
-        Self::only_err(ExpandError::Other(err.into()))
+    pub fn map<U>(self, f: impl FnOnce(T) -> U) -> ValueResult<U, E> {
+        ValueResult { value: f(self.value), err: self.err }
     }
 
-    pub fn map<U>(self, f: impl FnOnce(T) -> U) -> ExpandResult<U> {
-        ExpandResult { value: f(self.value), err: self.err }
+    pub fn map_err<E2>(self, f: impl FnOnce(E) -> E2) -> ValueResult<T, E2> {
+        ValueResult { value: self.value, err: self.err.map(f) }
     }
 
-    pub fn result(self) -> Result<T, ExpandError> {
+    pub fn result(self) -> Result<T, E> {
         self.err.map_or(Ok(self.value), Err)
     }
 }
 
-impl<T: Default> From<Result<T, ExpandError>> for ExpandResult<T> {
-    fn from(result: Result<T, ExpandError>) -> Self {
+impl<T: Default, E> From<Result<T, E>> for ValueResult<T, E> {
+    fn from(result: Result<T, E>) -> Self {
         result.map_or_else(Self::only_err, Self::ok)
     }
 }
diff --git a/crates/mbe/src/tt_iter.rs b/crates/mbe/src/tt_iter.rs
index 3a006a5a10..fc5590b718 100644
--- a/crates/mbe/src/tt_iter.rs
+++ b/crates/mbe/src/tt_iter.rs
@@ -106,7 +106,7 @@ impl<'a> TtIter<'a> {
         }
 
         let err = if error || !cursor.is_root() {
-            Some(ExpandError::BindingError(format!("expected {entry_point:?}").into()))
+            Some(ExpandError::binding_error(format!("expected {entry_point:?}")))
         } else {
             None
         };