Skip to content

fix: Strip unused token ids from eager macro input token maps #15367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions crates/hir-def/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ pub trait AsMacroCall {
&self,
db: &dyn ExpandDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
) -> Option<MacroCallId> {
self.as_call_id_with_errors(db, krate, resolver).ok()?.value
}
Expand All @@ -1092,7 +1092,7 @@ pub trait AsMacroCall {
&self,
db: &dyn ExpandDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro>;
}

Expand All @@ -1101,7 +1101,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
&self,
db: &dyn ExpandDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
let expands_to = hir_expand::ExpandTo::from_call_site(self.value);
let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
Expand All @@ -1112,12 +1112,13 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
return Ok(ExpandResult::only_err(ExpandError::other("malformed macro invocation")));
};

macro_call_as_call_id_(
macro_call_as_call_id_with_eager(
db,
&AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
expands_to,
krate,
resolver,
resolver,
)
}
}
Expand All @@ -1140,17 +1141,19 @@ fn macro_call_as_call_id(
call: &AstIdWithPath<ast::MacroCall>,
expand_to: ExpandTo,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
) -> Result<Option<MacroCallId>, UnresolvedMacro> {
macro_call_as_call_id_(db, call, expand_to, krate, resolver).map(|res| res.value)
macro_call_as_call_id_with_eager(db, call, expand_to, krate, resolver, resolver)
.map(|res| res.value)
}

fn macro_call_as_call_id_(
fn macro_call_as_call_id_with_eager(
db: &dyn ExpandDatabase,
call: &AstIdWithPath<ast::MacroCall>,
expand_to: ExpandTo,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
resolver: impl FnOnce(path::ModPath) -> Option<MacroDefId>,
eager_resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
let def =
resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
Expand All @@ -1159,8 +1162,8 @@ fn macro_call_as_call_id_(
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, &|path| {
resolver(path).filter(MacroDefId::is_fn_like)
})?
eager_resolver(path).filter(MacroDefId::is_fn_like)
})
}
_ if def.is_fn_like() => ExpandResult {
value: Some(def.as_lazy_macro(
Expand Down
24 changes: 24 additions & 0 deletions crates/hir-def/src/macro_expansion_tests/mbe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,30 @@ fn#19 main#20(#21)#21 {#22
);
}

#[test]
fn eager_expands_with_unresolved_within() {
check(
r#"
#[rustc_builtin_macro]
#[macro_export]
macro_rules! format_args {}

fn main(foo: ()) {
format_args!("{} {} {}", format_args!("{}", 0), foo, identity!(10), "bar")
}
"#,
expect![[r##"
#[rustc_builtin_macro]
#[macro_export]
macro_rules! format_args {}

fn main(foo: ()) {
/* error: unresolved macro identity */::core::fmt::Arguments::new_v1(&["", " ", " ", ], &[::core::fmt::ArgumentV1::new(&(::core::fmt::Arguments::new_v1(&["", ], &[::core::fmt::ArgumentV1::new(&(0), ::core::fmt::Display::fmt), ])), ::core::fmt::Display::fmt), ::core::fmt::ArgumentV1::new(&(foo), ::core::fmt::Display::fmt), ::core::fmt::ArgumentV1::new(&(identity!(10)), ::core::fmt::Display::fmt), ])
}
"##]],
);
}

#[test]
fn token_mapping_eager() {
check(
Expand Down
39 changes: 27 additions & 12 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::{
self, ExternCrate, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode,
MacroCall, MacroDef, MacroRules, Mod, ModItem, ModKind, TreeId,
},
macro_call_as_call_id, macro_id_to_def_id,
macro_call_as_call_id, macro_call_as_call_id_with_eager, macro_id_to_def_id,
nameres::{
diagnostics::DefDiagnostic,
mod_resolution::ModDir,
Expand Down Expand Up @@ -2187,7 +2187,7 @@ impl ModCollector<'_, '_> {
// scopes without eager expansion.

// Case 1: try to resolve macro calls with single-segment name and expand macro_rules
if let Ok(res) = macro_call_as_call_id(
if let Ok(res) = macro_call_as_call_id_with_eager(
db.upcast(),
&ast_id,
mac.expand_to,
Expand All @@ -2210,19 +2210,34 @@ impl ModCollector<'_, '_> {
.map(|it| macro_id_to_def_id(self.def_collector.db, it))
})
},
) {
// Legacy macros need to be expanded immediately, so that any macros they produce
// are in scope.
if let Some(val) = res {
self.def_collector.collect_macro_expansion(
|path| {
let resolved_res = self.def_collector.def_map.resolve_path_fp_with_macro(
db,
ResolveMode::Other,
self.module_id,
val,
self.macro_depth + 1,
container,
&path,
BuiltinShadowMode::Module,
Some(MacroSubNs::Bang),
);
}
resolved_res.resolved_def.take_macros().map(|it| macro_id_to_def_id(db, it))
},
) {
// FIXME: if there were errors, this mightve been in the eager expansion from an
// unresolved macro, so we need to push this into late macro resolution. see fixme above
if res.err.is_none() {
// Legacy macros need to be expanded immediately, so that any macros they produce
// are in scope.
if let Some(val) = res.value {
self.def_collector.collect_macro_expansion(
self.module_id,
val,
self.macro_depth + 1,
container,
);
}

return;
return;
}
}

// Case 2: resolve in module scope, expand during name resolution.
Expand Down
45 changes: 26 additions & 19 deletions crates/hir-expand/src/eager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//!
//! See the full discussion : <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Eager.20expansion.20of.20built-in.20macros>
use base_db::CrateId;
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use syntax::{ted, Parse, SyntaxNode, TextRange, TextSize, WalkEvent};
use triomphe::Arc;

Expand All @@ -29,7 +29,7 @@ use crate::{
hygiene::Hygiene,
mod_path::ModPath,
EagerCallInfo, ExpandError, ExpandResult, ExpandTo, InFile, MacroCallId, MacroCallKind,
MacroCallLoc, MacroDefId, MacroDefKind, UnresolvedMacro,
MacroCallLoc, MacroDefId, MacroDefKind,
};

pub fn expand_eager_macro_input(
Expand All @@ -38,7 +38,7 @@ pub fn expand_eager_macro_input(
macro_call: InFile<ast::MacroCall>,
def: MacroDefId,
resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
) -> ExpandResult<Option<MacroCallId>> {
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(&macro_call.value));
Expand Down Expand Up @@ -71,22 +71,23 @@ pub fn expand_eager_macro_input(
InFile::new(arg_id.as_file(), arg_exp.syntax_node()),
krate,
resolver,
)?
)
};
let err = parse_err.or(err);

let Some((expanded_eager_input, mapping)) = expanded_eager_input else {
return Ok(ExpandResult { value: None, err });
return ExpandResult { value: None, err };
};

let (mut subtree, expanded_eager_input_token_map) =
mbe::syntax_node_to_token_tree(&expanded_eager_input);

let og_tmap = if let Some(tt) = macro_call.value.token_tree() {
let og_tmap = mbe::syntax_node_to_token_map(tt.syntax());
let mut ids_used = FxHashSet::default();
let mut og_tmap = mbe::syntax_node_to_token_map(tt.syntax());
// The tokenmap and ids of subtree point into the expanded syntax node, but that is inaccessible from the outside
// so we need to remap them to the original input of the eager macro.
subtree.visit_ids(&|id| {
subtree.visit_ids(&mut |id| {
// Note: we discard all token ids of braces and the like here, but that's not too bad and only a temporary fix

if let Some(range) = expanded_eager_input_token_map
Expand All @@ -97,13 +98,15 @@ pub fn expand_eager_macro_input(
// remap from eager input expansion to original eager input
if let Some(&og_range) = ws_mapping.get(og_range) {
if let Some(og_token) = og_tmap.token_by_range(og_range) {
ids_used.insert(og_token);
return og_token;
}
}
}
}
tt::TokenId::UNSPECIFIED
});
og_tmap.filter(|id| ids_used.contains(&id));
og_tmap
} else {
Default::default()
Expand All @@ -121,7 +124,7 @@ pub fn expand_eager_macro_input(
kind: MacroCallKind::FnLike { ast_id: call_id, expand_to },
};

Ok(ExpandResult { value: Some(db.intern_macro_call(loc)), err })
ExpandResult { value: Some(db.intern_macro_call(loc)), err }
}

fn lazy_expand(
Expand All @@ -147,13 +150,13 @@ fn eager_macro_recur(
curr: InFile<SyntaxNode>,
krate: CrateId,
macro_resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,
) -> Result<ExpandResult<Option<(SyntaxNode, FxHashMap<TextRange, TextRange>)>>, UnresolvedMacro> {
) -> ExpandResult<Option<(SyntaxNode, FxHashMap<TextRange, TextRange>)>> {
let original = curr.value.clone_for_update();
let mut mapping = FxHashMap::default();

let mut replacements = Vec::new();

// Note: We only report a single error inside of eager expansions
// FIXME: We only report a single error inside of eager expansions
let mut error = None;
let mut offset = 0i32;
let apply_offset = |it: TextSize, offset: i32| {
Expand Down Expand Up @@ -184,24 +187,28 @@ fn eager_macro_recur(
}
};
let def = match call.path().and_then(|path| ModPath::from_src(db, path, hygiene)) {
Some(path) => macro_resolver(path.clone()).ok_or(UnresolvedMacro { path })?,
Some(path) => match macro_resolver(path.clone()) {
Some(def) => def,
None => {
error =
Some(ExpandError::other(format!("unresolved macro {}", path.display(db))));
continue;
}
},
None => {
error = Some(ExpandError::other("malformed macro invocation"));
continue;
}
};
let ExpandResult { value, err } = match def.kind {
MacroDefKind::BuiltInEager(..) => {
let ExpandResult { value, err } = match expand_eager_macro_input(
let ExpandResult { value, err } = expand_eager_macro_input(
db,
krate,
curr.with_value(call.clone()),
def,
macro_resolver,
) {
Ok(it) => it,
Err(err) => return Err(err),
};
);
match value {
Some(call_id) => {
let ExpandResult { value, err: err2 } =
Expand Down Expand Up @@ -251,7 +258,7 @@ fn eager_macro_recur(
parse.as_ref().map(|it| it.syntax_node()),
krate,
macro_resolver,
)?;
);
let err = err.or(error);

if let Some(tt) = call.token_tree() {
Expand Down Expand Up @@ -281,7 +288,7 @@ fn eager_macro_recur(
}
// check if the whole original syntax is replaced
if call.syntax() == &original {
return Ok(ExpandResult { value: value.zip(Some(mapping)), err: error });
return ExpandResult { value: value.zip(Some(mapping)), err: error };
}

if let Some(insert) = value {
Expand All @@ -292,5 +299,5 @@ fn eager_macro_recur(
}

replacements.into_iter().rev().for_each(|(old, new)| ted::replace(old.syntax(), new));
Ok(ExpandResult { value: Some((original, mapping)), err: error })
ExpandResult { value: Some((original, mapping)), err: error }
}
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/macro_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ macro_rules! m {

fn f() {
m!();
//^^^^ error: unresolved macro `$crate::private::concat!`
//^^^^ error: unresolved macro $crate::private::concat
}

//- /core.rs crate:core
Expand Down
14 changes: 12 additions & 2 deletions crates/ide/src/syntax_highlighting/test_data/highlight_macros.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,18 @@
<span class="brace">}</span>
<span class="brace">}</span>

<span class="attribute_bracket attribute">#</span><span class="attribute_bracket attribute">[</span><span class="builtin_attr attribute library">rustc_builtin_macro</span><span class="attribute_bracket attribute">]</span>
<span class="keyword">macro_rules</span><span class="macro_bang">!</span> <span class="macro declaration">concat</span> <span class="brace">{</span><span class="brace">}</span>
<span class="attribute_bracket attribute">#</span><span class="attribute_bracket attribute">[</span><span class="builtin_attr attribute library">rustc_builtin_macro</span><span class="attribute_bracket attribute">]</span>
<span class="keyword">macro_rules</span><span class="macro_bang">!</span> <span class="macro declaration">include</span> <span class="brace">{</span><span class="brace">}</span>
<span class="attribute_bracket attribute">#</span><span class="attribute_bracket attribute">[</span><span class="builtin_attr attribute library">rustc_builtin_macro</span><span class="attribute_bracket attribute">]</span>
<span class="keyword">macro_rules</span><span class="macro_bang">!</span> <span class="macro declaration">format_args</span> <span class="brace">{</span><span class="brace">}</span>

<span class="macro">include</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="none macro">concat</span><span class="punctuation macro">!</span><span class="parenthesis macro">(</span><span class="string_literal macro">"foo/"</span><span class="comma macro">,</span> <span class="string_literal macro">"foo.rs"</span><span class="parenthesis macro">)</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>

<span class="keyword">fn</span> <span class="function declaration">main</span><span class="parenthesis">(</span><span class="parenthesis">)</span> <span class="brace">{</span>
<span class="unresolved_reference">println</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="string_literal macro">"Hello, {}!"</span><span class="comma macro">,</span> <span class="numeric_literal macro">92</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
<span class="macro">format_args</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="string_literal macro">"Hello, </span><span class="format_specifier">{</span><span class="format_specifier">}</span><span class="string_literal macro">!"</span><span class="comma macro">,</span> <span class="numeric_literal macro">92</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
<span class="macro">dont_color_me_braces</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
<span class="macro">noop</span><span class="macro_bang">!</span><span class="parenthesis macro">(</span><span class="macro macro">noop</span><span class="macro_bang macro">!</span><span class="parenthesis macro">(</span><span class="numeric_literal macro">1</span><span class="parenthesis macro">)</span><span class="parenthesis macro">)</span><span class="semicolon">;</span>
<span class="brace">}</span></code></pre>
<span class="brace">}</span>
</code></pre>
Loading