Merge #9960

9960: internal: Use ExpandResult in all TokenExpanders r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir_expand/src/builtin_attr.rs

@@ -1,5 +1,6 @@
 //! Builtin attributes.
 
+use mbe::ExpandResult;
 use syntax::ast;
 
 use crate::{db::AstDatabase, name, AstId, CrateId, MacroCallId, MacroDefId, MacroDefKind};
@@ -18,7 +19,7 @@ macro_rules! register_builtin {
                 id: MacroCallId,
                 tt: &tt::Subtree,
                 item: &tt::Subtree,
-            ) -> Result<tt::Subtree, mbe::ExpandError> {
+            ) -> ExpandResult<tt::Subtree> {
                 let expander = match *self {
                     $( BuiltinAttrExpander::$variant => $expand, )*
                 };
@@ -64,6 +65,6 @@ fn dummy_attr_expand(
     _id: MacroCallId,
     _tt: &tt::Subtree,
     item: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
-    Ok(item.clone())
+) -> ExpandResult<tt::Subtree> {
+    ExpandResult::ok(item.clone())
 }

crates/hir_expand/src/builtin_derive.rs

@@ -2,6 +2,7 @@
 
 use log::debug;
 
+use mbe::ExpandResult;
 use parser::FragmentKind;
 use syntax::{
     ast::{self, AstNode, GenericParamsOwner, ModuleItemOwner, NameOwner},
@@ -23,7 +24,7 @@ macro_rules! register_builtin {
                 db: &dyn AstDatabase,
                 id: MacroCallId,
                 tt: &tt::Subtree,
-            ) -> Result<tt::Subtree, mbe::ExpandError> {
+            ) -> ExpandResult<tt::Subtree> {
                 let expander = match *self {
                     $( BuiltinDeriveExpander::$trait => $expand, )*
                 };
@@ -147,11 +148,11 @@ fn make_type_args(n: usize, bound: Vec<tt::TokenTree>) -> Vec<tt::TokenTree> {
     result
 }
 
-fn expand_simple_derive(
-    tt: &tt::Subtree,
-    trait_path: tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
-    let info = parse_adt(tt)?;
+fn expand_simple_derive(tt: &tt::Subtree, trait_path: tt::Subtree) -> ExpandResult<tt::Subtree> {
+    let info = match parse_adt(tt) {
+        Ok(info) => info,
+        Err(e) => return ExpandResult::only_err(e),
+    };
     let name = info.name;
     let trait_path_clone = trait_path.token_trees.clone();
     let bound = (quote! { : ##trait_path_clone }).token_trees;
@@ -161,7 +162,7 @@ fn expand_simple_derive(
     let expanded = quote! {
         impl ##type_params ##trait_path for #name ##type_args {}
     };
-    Ok(expanded)
+    ExpandResult::ok(expanded)
 }
 
 fn find_builtin_crate(db: &dyn AstDatabase, id: MacroCallId) -> tt::TokenTree {
@@ -186,7 +187,7 @@ fn copy_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::marker::Copy })
 }
@@ -195,7 +196,7 @@ fn clone_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::clone::Clone })
 }
@@ -204,7 +205,7 @@ fn default_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::default::Default })
 }
@@ -213,7 +214,7 @@ fn debug_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::fmt::Debug })
 }
@@ -222,16 +223,12 @@ fn hash_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::hash::Hash })
 }
 
-fn eq_expand(
-    db: &dyn AstDatabase,
-    id: MacroCallId,
-    tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+fn eq_expand(db: &dyn AstDatabase, id: MacroCallId, tt: &tt::Subtree) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::cmp::Eq })
 }
@@ -240,7 +237,7 @@ fn partial_eq_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::cmp::PartialEq })
 }
@@ -249,7 +246,7 @@ fn ord_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::cmp::Ord })
 }
@@ -258,7 +255,7 @@ fn partial_ord_expand(
     db: &dyn AstDatabase,
     id: MacroCallId,
     tt: &tt::Subtree,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+) -> ExpandResult<tt::Subtree> {
     let krate = find_builtin_crate(db, id);
     expand_simple_derive(tt, quote! { #krate::cmp::PartialOrd })
 }

crates/hir_expand/src/db.rs

@@ -53,19 +53,16 @@ impl TokenExpander {
             TokenExpander::MacroRules { mac, .. } => mac.expand(tt),
             TokenExpander::MacroDef { mac, .. } => mac.expand(tt),
             TokenExpander::Builtin(it) => it.expand(db, id, tt),
-            // FIXME switch these to ExpandResult as well
             TokenExpander::BuiltinAttr(it) => match db.macro_arg(id) {
-                Some(macro_arg) => it.expand(db, id, tt, &macro_arg.0).into(),
-                None => mbe::ExpandResult::only_err(
-                    mbe::ExpandError::Other("No item argument for attribute".to_string()).into(),
-                ),
+                Some(macro_arg) => it.expand(db, id, tt, &macro_arg.0),
+                None => mbe::ExpandResult::str_err("No item argument for attribute".to_string()),
             },
-            TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt).into(),
+            TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt),
             TokenExpander::ProcMacro(_) => {
                 // We store the result in salsa db to prevent non-deterministic behavior in
                 // some proc-macro implementation
                 // See #4315 for details
-                db.expand_proc_macro(id).into()
+                db.expand_proc_macro(id)
             }
         }
     }
@@ -133,7 +130,7 @@ pub trait AstDatabase: SourceDatabase {
     /// proc macros, since they are not deterministic in general, and
     /// non-determinism breaks salsa in a very, very, very bad way. @edwin0cheng
     /// heroically debugged this once!
-    fn expand_proc_macro(&self, call: MacroCallId) -> Result<tt::Subtree, mbe::ExpandError>;
+    fn expand_proc_macro(&self, call: MacroCallId) -> ExpandResult<tt::Subtree>;
     /// Firewall query that returns the error from the `macro_expand` query.
     fn macro_expand_error(&self, macro_call: MacroCallId) -> Option<ExpandError>;
 
@@ -379,18 +376,11 @@ fn macro_expand_error(db: &dyn AstDatabase, macro_call: MacroCallId) -> Option<E
     db.macro_expand(macro_call).err
 }
 
-fn expand_proc_macro(
-    db: &dyn AstDatabase,
-    id: MacroCallId,
-) -> Result<tt::Subtree, mbe::ExpandError> {
+fn expand_proc_macro(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult<tt::Subtree> {
     let loc: MacroCallLoc = db.lookup_intern_macro(id);
     let macro_arg = match db.macro_arg(id) {
         Some(it) => it,
-        None => {
-            return Err(
-                tt::ExpansionError::Unknown("No arguments for proc-macro".to_string()).into()
-            )
-        }
+        None => return ExpandResult::str_err("No arguments for proc-macro".to_string()),
     };
 
     let expander = match loc.def.kind {

crates/hir_expand/src/input.rs

@@ -62,8 +62,7 @@ fn remove_attr_invoc(item: ast::Item, attr_index: usize) -> ast::Item {
 
 #[cfg(test)]
 mod tests {
-    use base_db::fixture::WithFixture;
-    use base_db::SourceDatabase;
+    use base_db::{fixture::WithFixture, SourceDatabase};
     use expect_test::{expect, Expect};
 
     use crate::test_db::TestDB;

crates/hir_expand/src/proc_macro.rs

@@ -2,6 +2,7 @@
 
 use crate::db::AstDatabase;
 use base_db::{CrateId, ProcMacroId};
+use mbe::ExpandResult;
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
 pub struct ProcMacroExpander {
@@ -9,15 +10,6 @@ pub struct ProcMacroExpander {
     proc_macro_id: Option<ProcMacroId>,
 }
 
-macro_rules! err {
-    ($fmt:literal, $($tt:tt),*) => {
-        mbe::ExpandError::ProcMacroError(tt::ExpansionError::Unknown(format!($fmt, $($tt),*)))
-    };
-    ($fmt:literal) => {
-        mbe::ExpandError::ProcMacroError(tt::ExpansionError::Unknown($fmt.to_string()))
-    }
-}
-
 impl ProcMacroExpander {
     pub fn new(krate: CrateId, proc_macro_id: ProcMacroId) -> Self {
         Self { krate, proc_macro_id: Some(proc_macro_id) }
@@ -38,21 +30,21 @@ impl ProcMacroExpander {
         calling_crate: CrateId,
         tt: &tt::Subtree,
         attr_arg: Option<&tt::Subtree>,
-    ) -> Result<tt::Subtree, mbe::ExpandError> {
+    ) -> ExpandResult<tt::Subtree> {
         match self.proc_macro_id {
             Some(id) => {
                 let krate_graph = db.crate_graph();
-                let proc_macro = krate_graph[self.krate]
-                    .proc_macro
-                    .get(id.0 as usize)
-                    .ok_or_else(|| err!("No derive macro found."))?;
+                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 derive macro found.".to_string()),
+                };
 
                 // Proc macros have access to the environment variables of the invoking crate.
                 let env = &krate_graph[calling_crate].env;
 
-                proc_macro.expander.expand(tt, attr_arg, env).map_err(mbe::ExpandError::from)
+                proc_macro.expander.expand(tt, attr_arg, env).map_err(mbe::ExpandError::from).into()
             }
-            None => Err(mbe::ExpandError::UnresolvedProcMacro),
+            None => ExpandResult::only_err(mbe::ExpandError::UnresolvedProcMacro),
         }
     }
 }