From 6a9338e979ed90d2c0342db2d489c37bebb62ce7 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 26 Nov 2020 16:48:17 +0100 Subject: Use `ExpandResult` instead of `MacroResult` `MacroResult` is redundant --- crates/hir/src/lib.rs | 2 +- crates/hir_expand/src/db.rs | 80 ++++++++++++++++---------------------------- crates/hir_expand/src/lib.rs | 2 ++ crates/ide/src/status.rs | 6 ++-- crates/mbe/src/lib.rs | 8 +++++ 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 { - /// The result of the expansion. Might be `None` when error recovery was impossible and no - /// usable result was produced. - pub value: Option, - - /// 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, -} - #[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, Arc)>; - fn macro_expand(&self, macro_call: MacroCallId) -> MacroResult>; + ) -> ExpandResult, Arc)>>; + fn macro_expand(&self, macro_call: MacroCallId) -> ExpandResult>>; #[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; } -impl MacroResult { - fn error(message: String) -> Self { - Self { value: None, error: Some(message) } - } - - fn map(self, f: impl FnOnce(T) -> U) -> MacroResult { - MacroResult { value: self.value.map(f), error: self.error } - } - - fn drop_value(self) -> MacroResult { - 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 MacroResult> { +fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult>> { macro_expand_with_arg(db, id, None) } @@ -215,18 +188,18 @@ fn macro_expand_with_arg( db: &dyn AstDatabase, id: MacroCallId, arg: Option>, -) -> MacroResult> { +) -> ExpandResult>> { 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, ¯o_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 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 MacroResult<(Parse, Arc)> { +) -> ExpandResult, Arc)>> { 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>, -) -> MacroResult<(Parse, Arc)> { +) -> ExpandResult, Arc)>> { 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>> for SyntaxTreeStat } } -impl FromIterator, M)>>> +impl FromIterator, M)>>>> for SyntaxTreeStats { fn from_iter(iter: T) -> SyntaxTreeStats where - T: IntoIterator, M)>>>, + T: IntoIterator, 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 for ExpandError { @@ -264,6 +265,13 @@ impl ExpandResult { 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(self, f: impl FnOnce(T) -> U) -> ExpandResult { ExpandResult { value: f(self.value), err: self.err } } -- cgit v1.2.3