From 96a774261f4308532f813149d2e5677310555520 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 16 Apr 2023 19:20:42 +0200 Subject: [PATCH] Option begone part 1 --- .../hir-def/src/macro_expansion_tests/mod.rs | 64 +++++----- crates/hir-def/src/nameres/collector.rs | 2 +- crates/hir-expand/src/db.rs | 112 ++++++++---------- crates/hir-expand/src/fixup.rs | 4 +- crates/hir-expand/src/hygiene.rs | 4 +- crates/hir-expand/src/lib.rs | 4 +- crates/ide/src/status.rs | 4 +- .../rust-analyzer/src/cli/analysis_stats.rs | 5 +- 8 files changed, 94 insertions(+), 105 deletions(-) diff --git a/crates/hir-def/src/macro_expansion_tests/mod.rs b/crates/hir-def/src/macro_expansion_tests/mod.rs index 73a495d89bf..6286295c526 100644 --- a/crates/hir-def/src/macro_expansion_tests/mod.rs +++ b/crates/hir-def/src/macro_expansion_tests/mod.rs @@ -151,47 +151,45 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream if let Some(err) = exp.err { format_to!(expn_text, "/* error: {} */", err); } - if let Some((parse, token_map)) = exp.value { - if expect_errors { - assert!(!parse.errors().is_empty(), "no parse errors in expansion"); - for e in parse.errors() { - format_to!(expn_text, "/* parse error: {} */\n", e); - } - } else { - assert!( - parse.errors().is_empty(), - "parse errors in expansion: \n{:#?}", - parse.errors() - ); + let (parse, token_map) = exp.value; + if expect_errors { + assert!(!parse.errors().is_empty(), "no parse errors in expansion"); + for e in parse.errors() { + format_to!(expn_text, "/* parse error: {} */\n", e); } - let pp = pretty_print_macro_expansion( - parse.syntax_node(), - show_token_ids.then_some(&*token_map), + } else { + assert!( + parse.errors().is_empty(), + "parse errors in expansion: \n{:#?}", + parse.errors() ); - let indent = IndentLevel::from_node(call.syntax()); - let pp = reindent(indent, pp); - format_to!(expn_text, "{}", pp); + } + let pp = pretty_print_macro_expansion( + parse.syntax_node(), + show_token_ids.then_some(&*token_map), + ); + let indent = IndentLevel::from_node(call.syntax()); + let pp = reindent(indent, pp); + format_to!(expn_text, "{}", pp); - if tree { - let tree = format!("{:#?}", parse.syntax_node()) - .split_inclusive('\n') - .map(|line| format!("// {line}")) - .collect::(); - format_to!(expn_text, "\n{}", tree) - } + if tree { + let tree = format!("{:#?}", parse.syntax_node()) + .split_inclusive('\n') + .map(|line| format!("// {line}")) + .collect::(); + format_to!(expn_text, "\n{}", tree) } let range = call.syntax().text_range(); let range: Range = range.into(); if show_token_ids { - if let Some((tree, map, _)) = arg.as_deref() { - let tt_range = call.token_tree().unwrap().syntax().text_range(); - let mut ranges = Vec::new(); - extract_id_ranges(&mut ranges, map, tree); - for (range, id) in ranges { - let idx = (tt_range.start() + range.end()).into(); - text_edits.push((idx..idx, format!("#{}", id.0))); - } + let (tree, map, _) = &*arg; + let tt_range = call.token_tree().unwrap().syntax().text_range(); + let mut ranges = Vec::new(); + extract_id_ranges(&mut ranges, map, tree); + for (range, id) in ranges { + let idx = (tt_range.start() + range.end()).into(); + text_edits.push((idx..idx, format!("#{}", id.0))); } text_edits.push((range.start..range.start, "// ".into())); call.to_string().match_indices('\n').for_each(|(offset, _)| { diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 6592c6b9024..461b498fa01 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1371,7 +1371,7 @@ impl DefCollector<'_> { self.def_map.diagnostics.push(diag); } - if let Some(errors) = value { + if let errors @ [_, ..] = &*value { let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id); let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, &errors); self.def_map.diagnostics.push(diag); diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index afc2be07418..4794ccc2aae 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -108,7 +108,7 @@ pub trait ExpandDatabase: SourceDatabase { fn parse_macro_expansion( &self, macro_file: MacroFile, - ) -> ExpandResult, Arc)>>; + ) -> ExpandResult<(Parse, Arc)>; /// Macro ids. That's probably the tricksiest bit in rust-analyzer, and the /// reason why we use salsa at all. @@ -123,7 +123,7 @@ pub trait ExpandDatabase: SourceDatabase { fn macro_arg( &self, id: MacroCallId, - ) -> Option>; + ) -> Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupUndoInfo)>; /// Extracts syntax node, corresponding to a macro call. That's a firewall /// query, only typing in the macro call itself changes the returned /// subtree. @@ -133,7 +133,7 @@ pub trait ExpandDatabase: SourceDatabase { fn macro_def(&self, id: MacroDefId) -> Result, mbe::ParseError>; /// Expand macro call to a token tree. - fn macro_expand(&self, macro_call: MacroCallId) -> ExpandResult>>; + fn macro_expand(&self, macro_call: MacroCallId) -> ExpandResult>; /// Special case of the previous query for procedural macros. We can't LRU /// proc macros, since they are not deterministic in general, and /// non-determinism breaks salsa in a very, very, very bad way. @edwin0cheng @@ -143,7 +143,7 @@ pub trait ExpandDatabase: SourceDatabase { fn parse_macro_expansion_error( &self, macro_call: MacroCallId, - ) -> ExpandResult>>; + ) -> ExpandResult>; fn hygiene_frame(&self, file_id: HirFileId) -> Arc; } @@ -257,12 +257,12 @@ fn ast_id_map(db: &dyn ExpandDatabase, file_id: HirFileId) -> Arc { } fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> Option { - match file_id.repr() { - HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).tree().syntax().clone()), + Some(match file_id.repr() { + HirFileIdRepr::FileId(file_id) => db.parse(file_id).tree().syntax().clone(), HirFileIdRepr::MacroFile(macro_file) => { - db.parse_macro_expansion(macro_file).value.map(|(it, _)| it.syntax_node()) + db.parse_macro_expansion(macro_file).value.0.syntax_node() } - } + }) } fn parse_or_expand_with_err( @@ -272,7 +272,7 @@ fn parse_or_expand_with_err( match file_id.repr() { HirFileIdRepr::FileId(file_id) => ExpandResult::ok(Some(db.parse(file_id).to_syntax())), HirFileIdRepr::MacroFile(macro_file) => { - db.parse_macro_expansion(macro_file).map(|it| it.map(|(parse, _)| parse)) + db.parse_macro_expansion(macro_file).map(|it| Some(it.0)) } } } @@ -280,9 +280,9 @@ fn parse_or_expand_with_err( fn parse_macro_expansion( db: &dyn ExpandDatabase, macro_file: MacroFile, -) -> ExpandResult, Arc)>> { +) -> ExpandResult<(Parse, Arc)> { let _p = profile::span("parse_macro_expansion"); - let mbe::ValueResult { value, err } = db.macro_expand(macro_file.macro_call_id); + let mbe::ValueResult { value: tt, err } = db.macro_expand(macro_file.macro_call_id); if let Some(err) = &err { if tracing::enabled!(tracing::Level::DEBUG) { @@ -308,10 +308,6 @@ fn parse_macro_expansion( ); } } - let tt = match value { - Some(tt) => tt, - None => return ExpandResult { value: None, err }, - }; let expand_to = macro_expand_to(db, macro_file.macro_call_id); @@ -320,14 +316,23 @@ fn parse_macro_expansion( let (parse, rev_token_map) = token_tree_to_syntax_node(&tt, expand_to); - ExpandResult { value: Some((parse, Arc::new(rev_token_map))), err } + ExpandResult { value: (parse, Arc::new(rev_token_map)), err } } fn macro_arg( db: &dyn ExpandDatabase, id: MacroCallId, -) -> Option> { - let arg = db.macro_arg_text(id)?; +) -> Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupUndoInfo)> { + let Some(arg) = db.macro_arg_text(id) else { + return Arc::new(( + tt::Subtree { + delimiter: tt::Delimiter::UNSPECIFIED, + token_trees: Vec::new(), + }, + Default::default(), + Default::default()) + ); + }; let loc = db.lookup_intern_macro_call(id); let node = SyntaxNode::new_root(arg); @@ -346,7 +351,7 @@ fn macro_arg( // proc macros expect their inputs without parentheses, MBEs expect it with them included tt.delimiter = tt::Delimiter::unspecified(); } - Some(Arc::new((tt, tmap, fixups.undo_info))) + Arc::new((tt, tmap, fixups.undo_info)) } fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet { @@ -448,29 +453,13 @@ fn macro_def( } } -fn macro_expand( - db: &dyn ExpandDatabase, - id: MacroCallId, - // FIXME: Remove the OPtion if possible -) -> ExpandResult>> { +fn macro_expand(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult> { let _p = profile::span("macro_expand"); let loc: MacroCallLoc = db.lookup_intern_macro_call(id); if let Some(eager) = &loc.eager { - return ExpandResult { - value: Some(eager.arg_or_expansion.clone()), - err: eager.error.clone(), - }; + return ExpandResult { value: eager.arg_or_expansion.clone(), err: eager.error.clone() }; } - let macro_arg = match db.macro_arg(id) { - Some(it) => it, - None => { - return ExpandResult::only_err(ExpandError::Other( - "Failed to lower macro args to token tree".into(), - )) - } - }; - let expander = match db.macro_def(loc.def) { Ok(it) => it, // FIXME: This is weird -- we effectively report macro *definition* @@ -478,49 +467,52 @@ fn macro_expand( // be reported at the definition site when we construct a def map. // (Note we do report them also at the definition site in the late diagnostic pass) Err(err) => { - return ExpandResult::only_err(ExpandError::Other( - format!("invalid macro definition: {err}").into(), - )) + return ExpandResult { + value: Arc::new(tt::Subtree { + delimiter: tt::Delimiter::UNSPECIFIED, + token_trees: vec![], + }), + err: Some(ExpandError::Other(format!("invalid macro definition: {err}").into())), + } } }; + let macro_arg = db.macro_arg(id); let ExpandResult { value: mut tt, err } = expander.expand(db, id, ¯o_arg.0); // Set a hard limit for the expanded tt let count = tt.count(); if TOKEN_LIMIT.check(count).is_err() { - return ExpandResult::only_err(ExpandError::Other( - format!( - "macro invocation exceeds token limit: produced {} tokens, limit is {}", - count, - TOKEN_LIMIT.inner(), - ) - .into(), - )); + return ExpandResult { + value: Arc::new(tt::Subtree { + delimiter: tt::Delimiter::UNSPECIFIED, + token_trees: vec![], + }), + err: Some(ExpandError::Other( + format!( + "macro invocation exceeds token limit: produced {} tokens, limit is {}", + count, + TOKEN_LIMIT.inner(), + ) + .into(), + )), + }; } fixup::reverse_fixups(&mut tt, ¯o_arg.1, ¯o_arg.2); - ExpandResult { value: Some(Arc::new(tt)), err } + ExpandResult { value: Arc::new(tt), err } } fn parse_macro_expansion_error( db: &dyn ExpandDatabase, macro_call_id: MacroCallId, -) -> ExpandResult>> { +) -> ExpandResult> { db.parse_macro_expansion(MacroFile { macro_call_id }) - .map(|it| it.map(|(it, _)| it.errors().to_vec().into_boxed_slice())) + .map(|it| it.0.errors().to_vec().into_boxed_slice()) } fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult { let loc: MacroCallLoc = db.lookup_intern_macro_call(id); - let macro_arg = match db.macro_arg(id) { - Some(it) => it, - None => { - return ExpandResult::with_err( - tt::Subtree::empty(), - ExpandError::Other("No arguments for proc-macro".into()), - ) - } - }; + let macro_arg = db.macro_arg(id); let expander = match loc.def.kind { MacroDefKind::ProcMacro(expander, ..) => expander, diff --git a/crates/hir-expand/src/fixup.rs b/crates/hir-expand/src/fixup.rs index b273f21768c..00796e7c0db 100644 --- a/crates/hir-expand/src/fixup.rs +++ b/crates/hir-expand/src/fixup.rs @@ -14,7 +14,7 @@ use tt::token_id::Subtree; /// The result of calculating fixes for a syntax node -- a bunch of changes /// (appending to and replacing nodes), the information that is needed to /// reverse those changes afterwards, and a token map. -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct SyntaxFixups { pub(crate) append: FxHashMap>, pub(crate) replace: FxHashMap>, @@ -24,7 +24,7 @@ pub(crate) struct SyntaxFixups { } /// This is the information needed to reverse the fixups. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Default, PartialEq, Eq)] pub struct SyntaxFixupUndoInfo { original: Vec, } diff --git a/crates/hir-expand/src/hygiene.rs b/crates/hir-expand/src/hygiene.rs index 2eb56fc9e8b..addffb88772 100644 --- a/crates/hir-expand/src/hygiene.rs +++ b/crates/hir-expand/src/hygiene.rs @@ -200,8 +200,8 @@ fn make_hygiene_info( }); let macro_def = db.macro_def(loc.def).ok()?; - let (_, exp_map) = db.parse_macro_expansion(macro_file).value?; - let macro_arg = db.macro_arg(macro_file.macro_call_id)?; + let (_, exp_map) = db.parse_macro_expansion(macro_file).value; + let macro_arg = db.macro_arg(macro_file.macro_call_id); Some(HygieneInfo { file: macro_file, diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 9685320cf5d..f4d858e8e26 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -257,8 +257,8 @@ impl HirFileId { let arg_tt = loc.kind.arg(db)?; let macro_def = db.macro_def(loc.def).ok()?; - let (parse, exp_map) = db.parse_macro_expansion(macro_file).value?; - let macro_arg = db.macro_arg(macro_file.macro_call_id)?; + let (parse, exp_map) = db.parse_macro_expansion(macro_file).value; + let macro_arg = db.macro_arg(macro_file.macro_call_id); let def = loc.def.ast_id().left().and_then(|id| { let def_tt = match id.to_node(db) { diff --git a/crates/ide/src/status.rs b/crates/ide/src/status.rs index 7ce782f93be..71fc91cf317 100644 --- a/crates/ide/src/status.rs +++ b/crates/ide/src/status.rs @@ -120,12 +120,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/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index bd477775717..8aeb1df8d04 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -180,9 +180,8 @@ impl flags::AnalysisStats { let mut total_macro_file_size = Bytes::default(); for e in hir::db::ParseMacroExpansionQuery.in_db(db).entries::>() { - if let Some((val, _)) = db.parse_macro_expansion(e.key).value { - total_macro_file_size += syntax_len(val.syntax_node()) - } + let val = db.parse_macro_expansion(e.key).value.0; + total_macro_file_size += syntax_len(val.syntax_node()) } eprintln!("source files: {total_file_size}, macro files: {total_macro_file_size}"); }