From 87e0bbc534af5b4d372065e55bc5264fa4a5c920 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 13 Mar 2024 17:42:01 +0100 Subject: [PATCH 1/3] Stronger typing for macro_arg query --- crates/hir-expand/src/db.rs | 328 +++++++++++++++++------------------ crates/hir-expand/src/lib.rs | 1 + 2 files changed, 158 insertions(+), 171 deletions(-) diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index a7469ae5c88..632d8f2bcb8 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -6,19 +6,16 @@ use mbe::{syntax_node_to_token_tree, ValueResult}; use rustc_hash::FxHashSet; use span::{AstIdMap, SyntaxContextData, SyntaxContextId}; -use syntax::{ - ast::{self, HasAttrs}, - AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T, -}; +use syntax::{ast, AstNode, Parse, SyntaxElement, SyntaxError, SyntaxNode, SyntaxToken, T}; use triomphe::Arc; use crate::{ - attrs::collect_attrs, + attrs::{collect_attrs, AttrId}, builtin_attr_macro::pseudo_derive_attr_expansion, builtin_fn_macro::EagerExpander, cfg_process, declarative::DeclarativeMacroExpander, - fixup::{self, reverse_fixups, SyntaxFixupUndoInfo}, + fixup::{self, SyntaxFixupUndoInfo}, hygiene::{span_with_call_site_ctxt, span_with_def_site_ctxt, span_with_mixed_site_ctxt}, proc_macro::ProcMacros, span_map::{RealSpanMap, SpanMap, SpanMapRef}, @@ -149,8 +146,14 @@ pub fn expand_speculative( mbe::syntax_node_to_token_tree(speculative_args, span_map, loc.call_site), SyntaxFixupUndoInfo::NONE, ), - MacroCallKind::Derive { .. } | MacroCallKind::Attr { .. } => { - let censor = censor_for_macro_input(&loc, speculative_args); + MacroCallKind::Derive { derive_attr_index: index, .. } + | MacroCallKind::Attr { invoc_attr_index: index, .. } => { + let censor = if let MacroCallKind::Derive { .. } = loc.kind { + censor_derive_input(index, &ast::Adt::cast(speculative_args.clone())?) + } else { + censor_attr_input(index, &ast::Item::cast(speculative_args.clone())?) + }; + let censor_cfg = cfg_process::process_cfg_attrs(speculative_args, &loc, db).unwrap_or_default(); let mut fixups = fixup::fixup_syntax(span_map, speculative_args, loc.call_site); @@ -324,7 +327,8 @@ pub(crate) fn parse_with_map( } } -// FIXME: for derive attributes, this will return separate copies of the same structures! +// FIXME: for derive attributes, this will return separate copies of the same structures! Though +// they may differ in spans due to differing call sites... fn macro_arg( db: &dyn ExpandDatabase, id: MacroCallId, @@ -336,173 +340,155 @@ fn macro_arg( .then(|| loc.eager.as_deref()) .flatten() { - ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE)) - } else { - let (parse, map) = parse_with_map(db, loc.kind.file_id()); - let root = parse.syntax_node(); - - let syntax = match loc.kind { - MacroCallKind::FnLike { ast_id, .. } => { - let dummy_tt = |kind| { - ( - Arc::new(tt::Subtree { - delimiter: tt::Delimiter { - open: loc.call_site, - close: loc.call_site, - kind, - }, - token_trees: Box::default(), - }), - SyntaxFixupUndoInfo::default(), - ) - }; - - let node = &ast_id.to_ptr(db).to_node(&root); - let offset = node.syntax().text_range().start(); - let Some(tt) = node.token_tree() else { - return ValueResult::new( - dummy_tt(tt::DelimiterKind::Invisible), - Arc::new(Box::new([SyntaxError::new_at_offset( - "missing token tree".to_owned(), - offset, - )])), - ); - }; - let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']); - let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]); - - let mismatched_delimiters = !matches!( - (first, last), - (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']) - ); - if mismatched_delimiters { - // Don't expand malformed (unbalanced) macro invocations. This is - // less than ideal, but trying to expand unbalanced macro calls - // sometimes produces pathological, deeply nested code which breaks - // all kinds of things. - // - // So instead, we'll return an empty subtree here - cov_mark::hit!(issue9358_bad_macro_stack_overflow); - - let kind = match first { - _ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible, - T!['('] => tt::DelimiterKind::Parenthesis, - T!['['] => tt::DelimiterKind::Bracket, - T!['{'] => tt::DelimiterKind::Brace, - _ => tt::DelimiterKind::Invisible, - }; - return ValueResult::new( - dummy_tt(kind), - Arc::new(Box::new([SyntaxError::new_at_offset( - "mismatched delimiters".to_owned(), - offset, - )])), - ); - } - tt.syntax().clone() - } - MacroCallKind::Derive { ast_id, .. } => { - ast_id.to_ptr(db).to_node(&root).syntax().clone() - } - MacroCallKind::Attr { ast_id, .. } => ast_id.to_ptr(db).to_node(&root).syntax().clone(), - }; - let (mut tt, undo_info) = match loc.kind { - MacroCallKind::FnLike { .. } => ( - mbe::syntax_node_to_token_tree(&syntax, map.as_ref(), loc.call_site), - SyntaxFixupUndoInfo::NONE, - ), - MacroCallKind::Derive { .. } | MacroCallKind::Attr { .. } => { - let censor = censor_for_macro_input(&loc, &syntax); - let censor_cfg = - cfg_process::process_cfg_attrs(&syntax, &loc, db).unwrap_or_default(); - let mut fixups = fixup::fixup_syntax(map.as_ref(), &syntax, loc.call_site); - fixups.append.retain(|it, _| match it { - syntax::NodeOrToken::Token(_) => true, - it => !censor.contains(it) && !censor_cfg.contains(it), - }); - fixups.remove.extend(censor); - fixups.remove.extend(censor_cfg); - - { - let mut tt = mbe::syntax_node_to_token_tree_modified( - &syntax, - map.as_ref(), - fixups.append.clone(), - fixups.remove.clone(), - loc.call_site, - ); - reverse_fixups(&mut tt, &fixups.undo_info); - } - ( - mbe::syntax_node_to_token_tree_modified( - &syntax, - map, - fixups.append, - fixups.remove, - loc.call_site, - ), - fixups.undo_info, - ) - } - }; - - if loc.def.is_proc_macro() { - // proc macros expect their inputs without parentheses, MBEs expect it with them included - tt.delimiter.kind = tt::DelimiterKind::Invisible; - } - - if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) { - match parse.errors() { - errors if errors.is_empty() => ValueResult::ok((Arc::new(tt), undo_info)), - errors => ValueResult::new( - (Arc::new(tt), undo_info), - // Box::<[_]>::from(res.errors()), not stable yet - Arc::new(errors.to_vec().into_boxed_slice()), - ), - } - } else { - ValueResult::ok((Arc::new(tt), undo_info)) - } + return ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE)); } + + let (parse, map) = parse_with_map(db, loc.kind.file_id()); + let root = parse.syntax_node(); + + let (censor, item_node) = match loc.kind { + MacroCallKind::FnLike { ast_id, .. } => { + let dummy_tt = |kind| { + ( + Arc::new(tt::Subtree { + delimiter: tt::Delimiter { + open: loc.call_site, + close: loc.call_site, + kind, + }, + token_trees: Box::default(), + }), + SyntaxFixupUndoInfo::default(), + ) + }; + + let node = &ast_id.to_ptr(db).to_node(&root); + let offset = node.syntax().text_range().start(); + let Some(tt) = node.token_tree() else { + return ValueResult::new( + dummy_tt(tt::DelimiterKind::Invisible), + Arc::new(Box::new([SyntaxError::new_at_offset( + "missing token tree".to_owned(), + offset, + )])), + ); + }; + let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']); + let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]); + + let mismatched_delimiters = !matches!( + (first, last), + (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']) + ); + if mismatched_delimiters { + // Don't expand malformed (unbalanced) macro invocations. This is + // less than ideal, but trying to expand unbalanced macro calls + // sometimes produces pathological, deeply nested code which breaks + // all kinds of things. + // + // So instead, we'll return an empty subtree here + cov_mark::hit!(issue9358_bad_macro_stack_overflow); + + let kind = match first { + _ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible, + T!['('] => tt::DelimiterKind::Parenthesis, + T!['['] => tt::DelimiterKind::Bracket, + T!['{'] => tt::DelimiterKind::Brace, + _ => tt::DelimiterKind::Invisible, + }; + return ValueResult::new( + dummy_tt(kind), + Arc::new(Box::new([SyntaxError::new_at_offset( + "mismatched delimiters".to_owned(), + offset, + )])), + ); + } + + let tt = mbe::syntax_node_to_token_tree(tt.syntax(), map.as_ref(), loc.call_site); + let val = (Arc::new(tt), SyntaxFixupUndoInfo::NONE); + return if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) { + match parse.errors() { + errors if errors.is_empty() => ValueResult::ok(val), + errors => ValueResult::new( + val, + // Box::<[_]>::from(res.errors()), not stable yet + Arc::new(errors.to_vec().into_boxed_slice()), + ), + } + } else { + ValueResult::ok(val) + }; + } + MacroCallKind::Derive { ast_id, derive_attr_index, .. } => { + let node = ast_id.to_ptr(db).to_node(&root); + (censor_derive_input(derive_attr_index, &node), node.into()) + } + MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => { + let node = ast_id.to_ptr(db).to_node(&root); + (censor_attr_input(invoc_attr_index, &node), node) + } + }; + + let (mut tt, undo_info) = { + let syntax = item_node.syntax(); + let censor_cfg = cfg_process::process_cfg_attrs(syntax, &loc, db).unwrap_or_default(); + let mut fixups = fixup::fixup_syntax(map.as_ref(), syntax, loc.call_site); + fixups.append.retain(|it, _| match it { + syntax::NodeOrToken::Token(_) => true, + it => !censor.contains(it) && !censor_cfg.contains(it), + }); + fixups.remove.extend(censor); + fixups.remove.extend(censor_cfg); + + ( + mbe::syntax_node_to_token_tree_modified( + syntax, + map, + fixups.append, + fixups.remove, + loc.call_site, + ), + fixups.undo_info, + ) + }; + + if loc.def.is_proc_macro() { + // proc macros expect their inputs without parentheses, MBEs expect it with them included + tt.delimiter.kind = tt::DelimiterKind::Invisible; + } + + ValueResult::ok((Arc::new(tt), undo_info)) } // FIXME: Censoring info should be calculated by the caller! Namely by name resolution -/// Certain macro calls expect some nodes in the input to be preprocessed away, namely: -/// - derives expect all `#[derive(..)]` invocations up to the currently invoked one to be stripped -/// - attributes expect the invoking attribute to be stripped -fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet { +/// Derives expect all `#[derive(..)]` invocations up to the currently invoked one to be stripped +fn censor_derive_input(derive_attr_index: AttrId, node: &ast::Adt) -> FxHashSet { // FIXME: handle `cfg_attr` - (|| { - let censor = match loc.kind { - MacroCallKind::FnLike { .. } => return None, - MacroCallKind::Derive { derive_attr_index, .. } => { - cov_mark::hit!(derive_censoring); - ast::Item::cast(node.clone())? - .attrs() - .take(derive_attr_index.ast_index() + 1) - // FIXME, this resolution should not be done syntactically - // derive is a proper macro now, no longer builtin - // But we do not have resolution at this stage, this means - // we need to know about all macro calls for the given ast item here - // so we require some kind of mapping... - .filter(|attr| attr.simple_name().as_deref() == Some("derive")) - .map(|it| it.syntax().clone().into()) - .collect() - } - MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => return None, - MacroCallKind::Attr { invoc_attr_index, .. } => { - cov_mark::hit!(attribute_macro_attr_censoring); - collect_attrs(&ast::Item::cast(node.clone())?) - .nth(invoc_attr_index.ast_index()) - .and_then(|x| Either::left(x.1)) - .map(|attr| attr.syntax().clone().into()) - .into_iter() - .collect() - } - }; - Some(censor) - })() - .unwrap_or_default() + cov_mark::hit!(derive_censoring); + collect_attrs(node) + .take(derive_attr_index.ast_index() + 1) + .filter_map(|(_, attr)| Either::left(attr)) + // FIXME, this resolution should not be done syntactically + // derive is a proper macro now, no longer builtin + // But we do not have resolution at this stage, this means + // we need to know about all macro calls for the given ast item here + // so we require some kind of mapping... + .filter(|attr| attr.simple_name().as_deref() == Some("derive")) + .map(|it| it.syntax().clone().into()) + .collect() +} + +/// Attributes expect the invoking attribute to be stripped\ +fn censor_attr_input(invoc_attr_index: AttrId, node: &ast::Item) -> FxHashSet { + // FIXME: handle `cfg_attr` + cov_mark::hit!(attribute_macro_attr_censoring); + collect_attrs(node) + .nth(invoc_attr_index.ast_index()) + .and_then(|(_, attr)| Either::left(attr)) + .map(|attr| attr.syntax().clone().into()) + .into_iter() + .collect() } impl TokenExpander { diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 924f0da6bde..838cb4f38fc 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -176,6 +176,7 @@ pub struct MacroCallLoc { // leakage problems here eager: Option>, pub kind: MacroCallKind, + // FIXME: Spans while relative to an anchor, are still rather unstable pub call_site: Span, } impl_intern_value_trivial!(MacroCallLoc); From abe31774459740d12c1d25ec4a8a85a54ba96f60 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 13 Mar 2024 18:05:27 +0100 Subject: [PATCH 2/3] Shrink MacroCallLoc --- crates/hir-def/src/data.rs | 2 + crates/hir-def/src/lib.rs | 4 +- crates/hir-def/src/nameres/attr_resolution.rs | 4 +- crates/hir-def/src/nameres/collector.rs | 6 +- crates/hir-expand/src/db.rs | 57 +++++++++++-------- crates/hir-expand/src/eager.rs | 23 ++++++-- crates/hir-expand/src/lib.rs | 18 +++--- 7 files changed, 69 insertions(+), 45 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index d4c1db8b95b..f84852b2629 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -745,6 +745,7 @@ fn collect_item( self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike { ast_id: InFile::new(file_id, ast_id), expand_to: hir_expand::ExpandTo::Items, + eager: None, }); } Ok(None) => (), @@ -754,6 +755,7 @@ fn collect_item( MacroCallKind::FnLike { ast_id: InFile::new(file_id, ast_id), expand_to, + eager: None, }, Clone::clone(path), )); diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index d63f2268aa4..ae2959dff62 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -1410,10 +1410,10 @@ fn macro_call_as_call_id_with_eager( }) } _ if def.is_fn_like() => ExpandResult { - value: Some(def.as_lazy_macro( + value: Some(def.make_call( db, krate, - MacroCallKind::FnLike { ast_id: call.ast_id, expand_to }, + MacroCallKind::FnLike { ast_id: call.ast_id, expand_to, eager: None }, call_site, )), err: None, diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs index 1cadae8c87c..25744e9570e 100644 --- a/crates/hir-def/src/nameres/attr_resolution.rs +++ b/crates/hir-def/src/nameres/attr_resolution.rs @@ -116,7 +116,7 @@ pub(super) fn attr_macro_as_call_id( _ => None, }; - def.as_lazy_macro( + def.make_call( db.upcast(), krate, MacroCallKind::Attr { @@ -140,7 +140,7 @@ pub(super) fn derive_macro_as_call_id( let (macro_id, def_id) = resolver(item_attr.path.clone()) .filter(|(_, def_id)| def_id.is_derive()) .ok_or_else(|| UnresolvedMacro { path: item_attr.path.clone() })?; - let call_id = def_id.as_lazy_macro( + let call_id = def_id.make_call( db.upcast(), krate, MacroCallKind::Derive { diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index f9fe6d3b903..593a88af694 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1451,7 +1451,11 @@ fn finish(mut self) -> DefMap { if let Err(UnresolvedMacro { path }) = macro_call_as_call_id { self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call( directive.module_id, - MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: *expand_to }, + MacroCallKind::FnLike { + ast_id: ast_id.ast_id, + expand_to: *expand_to, + eager: None, + }, path, )); } diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 632d8f2bcb8..1639ce189e0 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -332,15 +332,16 @@ pub(crate) fn parse_with_map( fn macro_arg( db: &dyn ExpandDatabase, id: MacroCallId, - // FIXME: consider the following by putting fixup info into eager call info args - // ) -> ValueResult, Arc>> { ) -> ValueResult<(Arc, SyntaxFixupUndoInfo), Arc>> { let loc = db.lookup_intern_macro_call(id); - if let Some(EagerCallInfo { arg, .. }) = matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) - .then(|| loc.eager.as_deref()) - .flatten() + + if let MacroCallLoc { + def: MacroDefId { kind: MacroDefKind::BuiltInEager(..), .. }, + kind: MacroCallKind::FnLike { eager: Some(eager), .. }, + .. + } = &loc { - return ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE)); + return ValueResult::ok((eager.arg.clone(), SyntaxFixupUndoInfo::NONE)); } let (parse, map) = parse_with_map(db, loc.kind.file_id()); @@ -518,7 +519,7 @@ fn macro_expand( ) -> ExpandResult> { let _p = tracing::span!(tracing::Level::INFO, "macro_expand").entered(); - let ExpandResult { value: tt, mut err } = match loc.def.kind { + let ExpandResult { value: tt, err } = match loc.def.kind { MacroDefKind::ProcMacro(..) => return db.expand_proc_macro(macro_call_id).map(CowArc::Arc), _ => { let ValueResult { value: (macro_arg, undo_info), err } = db.macro_arg(macro_call_id); @@ -541,23 +542,34 @@ fn macro_expand( MacroDefKind::BuiltIn(it, _) => { it.expand(db, macro_call_id, arg).map_err(Into::into) } - // This might look a bit odd, but we do not expand the inputs to eager macros here. - // Eager macros inputs are expanded, well, eagerly when we collect the macro calls. - // That kind of expansion uses the ast id map of an eager macros input though which goes through - // the HirFileId machinery. As eager macro inputs are assigned a macro file id that query - // will end up going through here again, whereas we want to just want to inspect the raw input. - // As such we just return the input subtree here. - MacroDefKind::BuiltInEager(..) if loc.eager.is_none() => { - return ExpandResult { - value: CowArc::Arc(macro_arg.clone()), - err: err.map(format_parse_err), - }; - } MacroDefKind::BuiltInDerive(it, _) => { it.expand(db, macro_call_id, arg).map_err(Into::into) } MacroDefKind::BuiltInEager(it, _) => { - it.expand(db, macro_call_id, arg).map_err(Into::into) + // This might look a bit odd, but we do not expand the inputs to eager macros here. + // Eager macros inputs are expanded, well, eagerly when we collect the macro calls. + // That kind of expansion uses the ast id map of an eager macros input though which goes through + // the HirFileId machinery. As eager macro inputs are assigned a macro file id that query + // will end up going through here again, whereas we want to just want to inspect the raw input. + // As such we just return the input subtree here. + let eager = match &loc.kind { + MacroCallKind::FnLike { eager: None, .. } => { + return ExpandResult { + value: CowArc::Arc(macro_arg.clone()), + err: err.map(format_parse_err), + }; + } + MacroCallKind::FnLike { eager: Some(eager), .. } => Some(&**eager), + _ => None, + }; + + let mut res = it.expand(db, macro_call_id, arg).map_err(Into::into); + + if let Some(EagerCallInfo { error, .. }) = eager { + // FIXME: We should report both errors! + res.err = error.clone().or(res.err); + } + res } MacroDefKind::BuiltInAttr(it, _) => { let mut res = it.expand(db, macro_call_id, arg); @@ -574,11 +586,6 @@ fn macro_expand( } }; - if let Some(EagerCallInfo { error, .. }) = loc.eager.as_deref() { - // FIXME: We should report both errors! - err = error.clone().or(err); - } - // Skip checking token tree limit for include! macro call if !loc.def.is_include() { // Set a hard limit for the expanded tt diff --git a/crates/hir-expand/src/eager.rs b/crates/hir-expand/src/eager.rs index 5337a5bb028..e8ca4315eaf 100644 --- a/crates/hir-expand/src/eager.rs +++ b/crates/hir-expand/src/eager.rs @@ -40,7 +40,6 @@ pub fn expand_eager_macro_input( resolver: &dyn Fn(ModPath) -> Option, ) -> ExpandResult> { let ast_map = db.ast_id_map(macro_call.file_id); - // the expansion which the ast id map is built upon has no whitespace, so the offsets are wrong as macro_call is from the token tree that has whitespace! let call_id = InFile::new(macro_call.file_id, ast_map.ast_id(¯o_call.value)); let expand_to = ExpandTo::from_call_site(¯o_call.value); @@ -51,8 +50,7 @@ pub fn expand_eager_macro_input( let arg_id = MacroCallLoc { def, krate, - eager: None, - kind: MacroCallKind::FnLike { ast_id: call_id, expand_to: ExpandTo::Expr }, + kind: MacroCallKind::FnLike { ast_id: call_id, expand_to: ExpandTo::Expr, eager: None }, call_site, } .intern(db); @@ -89,8 +87,16 @@ pub fn expand_eager_macro_input( let loc = MacroCallLoc { def, krate, - eager: Some(Arc::new(EagerCallInfo { arg: Arc::new(subtree), arg_id, error: err.clone() })), - kind: MacroCallKind::FnLike { ast_id: call_id, expand_to }, + + kind: MacroCallKind::FnLike { + ast_id: call_id, + expand_to, + eager: Some(Arc::new(EagerCallInfo { + arg: Arc::new(subtree), + arg_id, + error: err.clone(), + })), + }, call_site, }; @@ -108,7 +114,12 @@ fn lazy_expand( let expand_to = ExpandTo::from_call_site(¯o_call.value); let ast_id = macro_call.with_value(ast_id); - let id = def.as_lazy_macro(db, krate, MacroCallKind::FnLike { ast_id, expand_to }, call_site); + let id = def.make_call( + db, + krate, + MacroCallKind::FnLike { ast_id, expand_to, eager: None }, + call_site, + ); let macro_file = id.as_macro_file(); db.parse_macro_expansion(macro_file) diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs index 838cb4f38fc..cf1bdce7ed3 100644 --- a/crates/hir-expand/src/lib.rs +++ b/crates/hir-expand/src/lib.rs @@ -170,11 +170,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { pub struct MacroCallLoc { pub def: MacroDefId, pub krate: CrateId, - /// Some if this is a macro call for an eager macro. Note that this is `None` - /// for the eager input macro file. - // FIXME: This is being interned, subtrees can vary quickly differ just slightly causing - // leakage problems here - eager: Option>, pub kind: MacroCallKind, // FIXME: Spans while relative to an anchor, are still rather unstable pub call_site: Span, @@ -215,6 +210,11 @@ pub enum MacroCallKind { FnLike { ast_id: AstId, expand_to: ExpandTo, + /// Some if this is a macro call for an eager macro. Note that this is `None` + /// for the eager input macro file. + // FIXME: This is being interned, subtrees can vary quickly differ just slightly causing + // leakage problems here + eager: Option>, }, Derive { ast_id: AstId, @@ -276,7 +276,7 @@ fn original_file_respecting_includes(mut self, db: &dyn ExpandDatabase) -> FileI HirFileIdRepr::MacroFile(file) => { let loc = db.lookup_intern_macro_call(file.macro_call_id); if loc.def.is_include() { - if let Some(eager) = &loc.eager { + if let MacroCallKind::FnLike { eager: Some(eager), .. } = &loc.kind { if let Ok(it) = builtin_fn_macro::include_input_to_file_id( db, file.macro_call_id, @@ -406,14 +406,14 @@ fn is_derive_attr_pseudo_expansion(&self, db: &dyn ExpandDatabase) -> bool { } impl MacroDefId { - pub fn as_lazy_macro( + pub fn make_call( self, db: &dyn ExpandDatabase, krate: CrateId, kind: MacroCallKind, call_site: Span, ) -> MacroCallId { - MacroCallLoc { def: self, krate, eager: None, kind, call_site }.intern(db) + MacroCallLoc { def: self, krate, kind, call_site }.intern(db) } pub fn definition_range(&self, db: &dyn ExpandDatabase) -> InFile { @@ -535,7 +535,7 @@ pub fn include_file_id( macro_call_id: MacroCallId, ) -> Option { if self.def.is_include() { - if let Some(eager) = &self.eager { + if let MacroCallKind::FnLike { eager: Some(eager), .. } = &self.kind { if let Ok(it) = builtin_fn_macro::include_input_to_file_id(db, macro_call_id, &eager.arg) { From 9767156a2980820efe738b3629df396901ff8d70 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 13 Mar 2024 18:47:56 +0100 Subject: [PATCH 3/3] Simplify --- crates/hir-def/src/lib.rs | 15 ++++++----- crates/hir-expand/src/builtin_attr_macro.rs | 4 +-- crates/hir-expand/src/db.rs | 10 +++++++- crates/hir-expand/src/eager.rs | 28 ++++++++++----------- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index ae2959dff62..977782dfaf3 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -1403,12 +1403,15 @@ fn macro_call_as_call_id_with_eager( resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?; let res = match def.kind { - MacroDefKind::BuiltInEager(..) => { - let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db)); - expand_eager_macro_input(db, krate, macro_call, def, call_site, &|path| { - eager_resolver(path).filter(MacroDefId::is_fn_like) - }) - } + MacroDefKind::BuiltInEager(..) => expand_eager_macro_input( + db, + krate, + &call.ast_id.to_node(db), + call.ast_id, + def, + call_site, + &|path| eager_resolver(path).filter(MacroDefId::is_fn_like), + ), _ if def.is_fn_like() => ExpandResult { value: Some(def.make_call( db, diff --git a/crates/hir-expand/src/builtin_attr_macro.rs b/crates/hir-expand/src/builtin_attr_macro.rs index a0102f36aff..64295f64dcd 100644 --- a/crates/hir-expand/src/builtin_attr_macro.rs +++ b/crates/hir-expand/src/builtin_attr_macro.rs @@ -117,7 +117,7 @@ fn derive_expand( } pub fn pseudo_derive_attr_expansion( - tt: &tt::Subtree, + _: &tt::Subtree, args: &tt::Subtree, call_site: Span, ) -> ExpandResult { @@ -141,7 +141,7 @@ pub fn pseudo_derive_attr_expansion( token_trees.push(mk_leaf(']')); } ExpandResult::ok(tt::Subtree { - delimiter: tt.delimiter, + delimiter: args.delimiter, token_trees: token_trees.into_boxed_slice(), }) } diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index 1639ce189e0..40cbb6912db 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -146,6 +146,10 @@ pub fn expand_speculative( mbe::syntax_node_to_token_tree(speculative_args, span_map, loc.call_site), SyntaxFixupUndoInfo::NONE, ), + MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => ( + mbe::syntax_node_to_token_tree(speculative_args, span_map, loc.call_site), + SyntaxFixupUndoInfo::NONE, + ), MacroCallKind::Derive { derive_attr_index: index, .. } | MacroCallKind::Attr { invoc_attr_index: index, .. } => { let censor = if let MacroCallKind::Derive { .. } = loc.kind { @@ -406,7 +410,11 @@ fn macro_arg( ); } - let tt = mbe::syntax_node_to_token_tree(tt.syntax(), map.as_ref(), loc.call_site); + let mut tt = mbe::syntax_node_to_token_tree(tt.syntax(), map.as_ref(), loc.call_site); + if loc.def.is_proc_macro() { + // proc macros expect their inputs without parentheses, MBEs expect it with them included + tt.delimiter.kind = tt::DelimiterKind::Invisible; + } let val = (Arc::new(tt), SyntaxFixupUndoInfo::NONE); return if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) { match parse.errors() { diff --git a/crates/hir-expand/src/eager.rs b/crates/hir-expand/src/eager.rs index e8ca4315eaf..4524463e63e 100644 --- a/crates/hir-expand/src/eager.rs +++ b/crates/hir-expand/src/eager.rs @@ -27,21 +27,20 @@ ast::{self, AstNode}, db::ExpandDatabase, mod_path::ModPath, - EagerCallInfo, ExpandError, ExpandResult, ExpandTo, ExpansionSpanMap, InFile, Intern, + AstId, EagerCallInfo, ExpandError, ExpandResult, ExpandTo, ExpansionSpanMap, InFile, Intern, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, }; pub fn expand_eager_macro_input( db: &dyn ExpandDatabase, krate: CrateId, - macro_call: InFile, + macro_call: &ast::MacroCall, + ast_id: AstId, def: MacroDefId, call_site: Span, resolver: &dyn Fn(ModPath) -> Option, ) -> ExpandResult> { - let ast_map = db.ast_id_map(macro_call.file_id); - let call_id = InFile::new(macro_call.file_id, ast_map.ast_id(¯o_call.value)); - let expand_to = ExpandTo::from_call_site(¯o_call.value); + let expand_to = ExpandTo::from_call_site(macro_call); // Note: // When `lazy_expand` is called, its *parent* file must already exist. @@ -50,7 +49,7 @@ pub fn expand_eager_macro_input( let arg_id = MacroCallLoc { def, krate, - kind: MacroCallKind::FnLike { ast_id: call_id, expand_to: ExpandTo::Expr, eager: None }, + kind: MacroCallKind::FnLike { ast_id, expand_to: ExpandTo::Expr, eager: None }, call_site, } .intern(db); @@ -87,9 +86,8 @@ pub fn expand_eager_macro_input( let loc = MacroCallLoc { def, krate, - kind: MacroCallKind::FnLike { - ast_id: call_id, + ast_id, expand_to, eager: Some(Arc::new(EagerCallInfo { arg: Arc::new(subtree), @@ -106,14 +104,12 @@ pub fn expand_eager_macro_input( fn lazy_expand( db: &dyn ExpandDatabase, def: &MacroDefId, - macro_call: InFile, + macro_call: &ast::MacroCall, + ast_id: AstId, krate: CrateId, call_site: Span, ) -> ExpandResult<(InFile>, Arc)> { - let ast_id = db.ast_id_map(macro_call.file_id).ast_id(¯o_call.value); - - let expand_to = ExpandTo::from_call_site(¯o_call.value); - let ast_id = macro_call.with_value(ast_id); + let expand_to = ExpandTo::from_call_site(macro_call); let id = def.make_call( db, krate, @@ -183,12 +179,14 @@ fn eager_macro_recur( continue; } }; + let ast_id = db.ast_id_map(curr.file_id).ast_id(&call); let ExpandResult { value, err } = match def.kind { MacroDefKind::BuiltInEager(..) => { let ExpandResult { value, err } = expand_eager_macro_input( db, krate, - curr.with_value(call.clone()), + &call, + curr.with_value(ast_id), def, call_site, macro_resolver, @@ -218,7 +216,7 @@ fn eager_macro_recur( | MacroDefKind::BuiltInDerive(..) | MacroDefKind::ProcMacro(..) => { let ExpandResult { value: (parse, tm), err } = - lazy_expand(db, &def, curr.with_value(call.clone()), krate, call_site); + lazy_expand(db, &def, &call, curr.with_value(ast_id), krate, call_site); // replace macro inside let ExpandResult { value, err: error } = eager_macro_recur(