Merge #6639

6639: Use `ExpandResult` instead of `MacroResult` r=jonas-schievink a=jonas-schievink

`MacroResult` is redundant.

bors r+ :robot:

Co-authored-by: Jonas Schievink <[email protected]>

5 files changed, 42 insertions, 56 deletions

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index ed110329d..93bdb4472 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -57,7 +57,7 @@ pub use hir_def::{
     visibility::Visibility,
 };
 pub use hir_expand::{
-    db::MacroResult, name::known, name::AsName, name::Name, HirFileId, InFile, MacroCallId,
+    name::known, name::AsName, name::Name, ExpandResult, HirFileId, InFile, MacroCallId,
     MacroCallLoc, /* FIXME */ MacroDefId, MacroFile, Origin,
 };
 pub use hir_ty::display::HirDisplay;
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 3deac1711..46ebdbc74 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -13,19 +13,6 @@ use crate::{
     MacroFile, ProcMacroExpander,
 };
 
-/// A result of some macro expansion.
-#[derive(Debug, Clone, Eq, PartialEq)]
-pub struct MacroResult<T> {
-    /// The result of the expansion. Might be `None` when error recovery was impossible and no
-    /// usable result was produced.
-    pub value: Option<T>,
-
-    /// The error that occurred during expansion or processing.
-    ///
-    /// Since we do error recovery, getting an error here does not mean that `value` will be absent.
-    pub error: Option<String>,
-}
-
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum TokenExpander {
     MacroRules(mbe::MacroRules),
@@ -91,8 +78,8 @@ pub trait AstDatabase: SourceDatabase {
     fn parse_macro_expansion(
         &self,
         macro_file: MacroFile,
-    ) -> MacroResult<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>;
-    fn macro_expand(&self, macro_call: MacroCallId) -> MacroResult<Arc<tt::Subtree>>;
+    ) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>>;
+    fn macro_expand(&self, macro_call: MacroCallId) -> ExpandResult<Option<Arc<tt::Subtree>>>;
 
     #[salsa::interned]
     fn intern_eager_expansion(&self, eager: EagerCallLoc) -> EagerMacroId;
@@ -100,20 +87,6 @@ pub trait AstDatabase: SourceDatabase {
     fn expand_proc_macro(&self, call: MacroCallId) -> Result<tt::Subtree, mbe::ExpandError>;
 }
 
-impl<T> MacroResult<T> {
-    fn error(message: String) -> Self {
-        Self { value: None, error: Some(message) }
-    }
-
-    fn map<U>(self, f: impl FnOnce(T) -> U) -> MacroResult<U> {
-        MacroResult { value: self.value.map(f), error: self.error }
-    }
-
-    fn drop_value<U>(self) -> MacroResult<U> {
-        MacroResult { value: None, error: self.error }
-    }
-}
-
 /// This expands the given macro call, but with different arguments. This is
 /// used for completion, where we want to see what 'would happen' if we insert a
 /// token. The `token_to_map` mapped down into the expansion, with the mapped
@@ -194,7 +167,7 @@ fn macro_arg(db: &dyn AstDatabase, id: MacroCallId) -> Option<Arc<(tt::Subtree,
     Some(Arc::new((tt, tmap)))
 }
 
-fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> MacroResult<Arc<tt::Subtree>> {
+fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult<Option<Arc<tt::Subtree>>> {
     macro_expand_with_arg(db, id, None)
 }
 
@@ -215,18 +188,18 @@ fn macro_expand_with_arg(
     db: &dyn AstDatabase,
     id: MacroCallId,
     arg: Option<Arc<(tt::Subtree, mbe::TokenMap)>>,
-) -> MacroResult<Arc<tt::Subtree>> {
+) -> ExpandResult<Option<Arc<tt::Subtree>>> {
     let lazy_id = match id {
         MacroCallId::LazyMacro(id) => id,
         MacroCallId::EagerMacro(id) => {
             if arg.is_some() {
-                return MacroResult::error(
+                return ExpandResult::str_err(
                     "hypothetical macro expansion not implemented for eager macro".to_owned(),
                 );
             } else {
-                return MacroResult {
+                return ExpandResult {
                     value: Some(db.lookup_intern_eager_expansion(id).subtree),
-                    error: None,
+                    err: None,
                 };
             }
         }
@@ -235,21 +208,24 @@ fn macro_expand_with_arg(
     let loc = db.lookup_intern_macro(lazy_id);
     let macro_arg = match arg.or_else(|| db.macro_arg(id)) {
         Some(it) => it,
-        None => return MacroResult::error("Fail to args in to tt::TokenTree".into()),
+        None => return ExpandResult::str_err("Fail to args in to tt::TokenTree".into()),
     };
 
     let macro_rules = match db.macro_def(loc.def) {
         Some(it) => it,
-        None => return MacroResult::error("Fail to find macro definition".into()),
+        None => return ExpandResult::str_err("Fail to find macro definition".into()),
     };
     let ExpandResult { value: tt, err } = macro_rules.0.expand(db, lazy_id, &macro_arg.0);
     // Set a hard limit for the expanded tt
     let count = tt.count();
     if count > 262144 {
-        return MacroResult::error(format!("Total tokens count exceed limit : count = {}", count));
+        return ExpandResult::str_err(format!(
+            "Total tokens count exceed limit : count = {}",
+            count
+        ));
     }
 
-    MacroResult { value: Some(Arc::new(tt)), error: err.map(|e| format!("{:?}", e)) }
+    ExpandResult { value: Some(Arc::new(tt)), err }
 }
 
 fn expand_proc_macro(
@@ -283,7 +259,7 @@ fn parse_or_expand(db: &dyn AstDatabase, file_id: HirFileId) -> Option<SyntaxNod
     match file_id.0 {
         HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).tree().syntax().clone()),
         HirFileIdRepr::MacroFile(macro_file) => {
-            db.parse_macro_expansion(macro_file).map(|(it, _)| it.syntax_node()).value
+            db.parse_macro_expansion(macro_file).value.map(|(it, _)| it.syntax_node())
         }
     }
 }
@@ -291,7 +267,7 @@ fn parse_or_expand(db: &dyn AstDatabase, file_id: HirFileId) -> Option<SyntaxNod
 fn parse_macro_expansion(
     db: &dyn AstDatabase,
     macro_file: MacroFile,
-) -> MacroResult<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)> {
+) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>> {
     parse_macro_with_arg(db, macro_file, None)
 }
 
@@ -299,7 +275,7 @@ fn parse_macro_with_arg(
     db: &dyn AstDatabase,
     macro_file: MacroFile,
     arg: Option<Arc<(tt::Subtree, mbe::TokenMap)>>,
-) -> MacroResult<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)> {
+) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>> {
     let _p = profile::span("parse_macro_query");
 
     let macro_call_id = macro_file.macro_call_id;
@@ -308,7 +284,7 @@ fn parse_macro_with_arg(
     } else {
         db.macro_expand(macro_call_id)
     };
-    if let Some(err) = &result.error {
+    if let Some(err) = &result.err {
         // Note:
         // The final goal we would like to make all parse_macro success,
         // such that the following log will not call anyway.
@@ -326,20 +302,20 @@ fn parse_macro_with_arg(
                 .join("\n");
 
                 log::warn!(
-                    "fail on macro_parse: (reason: {} macro_call: {:#}) parents: {}",
+                    "fail on macro_parse: (reason: {:?} macro_call: {:#}) parents: {}",
                     err,
                     node.value,
                     parents
                 );
             }
             _ => {
-                log::warn!("fail on macro_parse: (reason: {})", err);
+                log::warn!("fail on macro_parse: (reason: {:?})", err);
             }
         }
     }
     let tt = match result.value {
         Some(tt) => tt,
-        None => return result.drop_value(),
+        None => return ExpandResult { value: None, err: result.err },
     };
 
     let fragment_kind = to_fragment_kind(db, macro_call_id);
@@ -347,29 +323,29 @@ fn parse_macro_with_arg(
     let (parse, rev_token_map) = match mbe::token_tree_to_syntax_node(&tt, fragment_kind) {
         Ok(it) => it,
         Err(err) => {
-            return MacroResult::error(format!("{:?}", err));
+            return ExpandResult::only_err(err);
         }
     };
 
-    match result.error {
-        Some(error) => {
+    match result.err {
+        Some(err) => {
             // Safety check for recursive identity macro.
             let node = parse.syntax_node();
             let file: HirFileId = macro_file.into();
             let call_node = match file.call_node(db) {
                 Some(it) => it,
                 None => {
-                    return MacroResult::error(error);
+                    return ExpandResult::only_err(err);
                 }
             };
 
             if !diff(&node, &call_node.value).is_empty() {
-                MacroResult { value: Some((parse, Arc::new(rev_token_map))), error: Some(error) }
+                ExpandResult { value: Some((parse, Arc::new(rev_token_map))), err: Some(err) }
             } else {
-                return MacroResult::error(error);
+                return ExpandResult::only_err(err);
             }
         }
-        None => MacroResult { value: Some((parse, Arc::new(rev_token_map))), error: None },
+        None => ExpandResult { value: Some((parse, Arc::new(rev_token_map))), err: None },
     }
 }
 
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs
index 83e09738b..d5ba691b7 100644
--- a/crates/hir_expand/src/lib.rs
+++ b/crates/hir_expand/src/lib.rs
@@ -15,6 +15,8 @@ pub mod proc_macro;
 pub mod quote;
 pub mod eager;
 
+pub use mbe::{ExpandError, ExpandResult};
+
 use std::hash::Hash;
 use std::sync::Arc;
 
diff --git a/crates/ide/src/status.rs b/crates/ide/src/status.rs
index b75f88ed9..e10d7c3a4 100644
--- a/crates/ide/src/status.rs
+++ b/crates/ide/src/status.rs
@@ -1,6 +1,6 @@
 use std::{fmt, iter::FromIterator, sync::Arc};
 
-use hir::{MacroFile, MacroResult};
+use hir::{ExpandResult, MacroFile};
 use ide_db::base_db::{
     salsa::debug::{DebugQueryTable, TableEntry},
     CrateId, FileId, FileTextQuery, SourceDatabase, SourceRootId,
@@ -115,12 +115,12 @@ impl FromIterator<TableEntry<FileId, Parse<ast::SourceFile>>> for SyntaxTreeStat
     }
 }
 
-impl<M> FromIterator<TableEntry<MacroFile, MacroResult<(Parse<SyntaxNode>, M)>>>
+impl<M> FromIterator<TableEntry<MacroFile, ExpandResult<Option<(Parse<SyntaxNode>, M)>>>>
     for SyntaxTreeStats
 {
     fn from_iter<T>(iter: T) -> SyntaxTreeStats
     where
-        T: IntoIterator<Item = TableEntry<MacroFile, MacroResult<(Parse<SyntaxNode>, M)>>>,
+        T: IntoIterator<Item = TableEntry<MacroFile, ExpandResult<Option<(Parse<SyntaxNode>, M)>>>>,
     {
         let mut res = SyntaxTreeStats::default();
         for entry in iter {
diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs
index 183e3b988..22fbf9a80 100644
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@@ -33,6 +33,7 @@ pub enum ExpandError {
     ConversionError,
     InvalidRepeat,
     ProcMacroError(tt::ExpansionError),
+    Other(String),
 }
 
 impl From<tt::ExpansionError> for ExpandError {
@@ -264,6 +265,13 @@ impl<T> ExpandResult<T> {
         Self { value: Default::default(), err: Some(err) }
     }
 
+    pub fn str_err(err: String) -> Self
+    where
+        T: Default,
+    {
+        Self::only_err(ExpandError::Other(err))
+    }
+
     pub fn map<U>(self, f: impl FnOnce(T) -> U) -> ExpandResult<U> {
         ExpandResult { value: f(self.value), err: self.err }
     }