Skip to content

Commit 151c750

Browse files
borsbors
committed
Auto merge of #15367 - Veykril:eager-macro-inputs, r=Veykril
fix: Strip unused token ids from eager macro input token maps
2 parents 62dcf39 + a5059da commit 151c750

File tree

10 files changed

+123
-47
lines changed

10 files changed

+123
-47
lines changed

‎crates/hir-def/src/lib.rs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ pub trait AsMacroCall {
10831083
&self,
10841084
db: &dyn ExpandDatabase,
10851085
krate: CrateId,
1086-
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
1086+
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
10871087
) -> Option<MacroCallId> {
10881088
self.as_call_id_with_errors(db, krate, resolver).ok()?.value
10891089
}
@@ -1092,7 +1092,7 @@ pub trait AsMacroCall {
10921092
&self,
10931093
db: &dyn ExpandDatabase,
10941094
krate: CrateId,
1095-
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
1095+
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
10961096
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro>;
10971097
}
10981098

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

1115-
macro_call_as_call_id_(
1115+
macro_call_as_call_id_with_eager(
11161116
db,
11171117
&AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
11181118
expands_to,
11191119
krate,
11201120
resolver,
1121+
resolver,
11211122
)
11221123
}
11231124
}
@@ -1140,17 +1141,19 @@ fn macro_call_as_call_id(
11401141
call: &AstIdWithPath<ast::MacroCall>,
11411142
expand_to: ExpandTo,
11421143
krate: CrateId,
1143-
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
1144+
resolver: impl Fn(path::ModPath) -> Option<MacroDefId> + Copy,
11441145
) -> Result<Option<MacroCallId>, UnresolvedMacro> {
1145-
macro_call_as_call_id_(db, call, expand_to, krate, resolver).map(|res| res.value)
1146+
macro_call_as_call_id_with_eager(db, call, expand_to, krate, resolver, resolver)
1147+
.map(|res| res.value)
11461148
}
11471149

1148-
fn macro_call_as_call_id_(
1150+
fn macro_call_as_call_id_with_eager(
11491151
db: &dyn ExpandDatabase,
11501152
call: &AstIdWithPath<ast::MacroCall>,
11511153
expand_to: ExpandTo,
11521154
krate: CrateId,
1153-
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
1155+
resolver: impl FnOnce(path::ModPath) -> Option<MacroDefId>,
1156+
eager_resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
11541157
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
11551158
let def =
11561159
resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
@@ -1159,8 +1162,8 @@ fn macro_call_as_call_id_(
11591162
MacroDefKind::BuiltInEager(..) => {
11601163
let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db));
11611164
expand_eager_macro_input(db, krate, macro_call, def, &|path| {
1162-
resolver(path).filter(MacroDefId::is_fn_like)
1163-
})?
1165+
eager_resolver(path).filter(MacroDefId::is_fn_like)
1166+
})
11641167
}
11651168
_ if def.is_fn_like() => ExpandResult {
11661169
value: Some(def.as_lazy_macro(

‎crates/hir-def/src/macro_expansion_tests/mbe.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,30 @@ fn#19 main#20(#21)#21 {#22
9999
);
100100
}
101101

102+
#[test]
103+
fn eager_expands_with_unresolved_within() {
104+
check(
105+
r#"
106+
#[rustc_builtin_macro]
107+
#[macro_export]
108+
macro_rules! format_args {}
109+
110+
fn main(foo: ()) {
111+
format_args!("{} {} {}", format_args!("{}", 0), foo, identity!(10), "bar")
112+
}
113+
"#,
114+
expect![[r##"
115+
#[rustc_builtin_macro]
116+
#[macro_export]
117+
macro_rules! format_args {}
118+
119+
fn main(foo: ()) {
120+
/* 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), ])
121+
}
122+
"##]],
123+
);
124+
}
125+
102126
#[test]
103127
fn token_mapping_eager() {
104128
check(

‎crates/hir-def/src/nameres/collector.rs‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::{
3838
self, ExternCrate, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode,
3939
MacroCall, MacroDef, MacroRules, Mod, ModItem, ModKind, TreeId,
4040
},
41-
macro_call_as_call_id, macro_id_to_def_id,
41+
macro_call_as_call_id, macro_call_as_call_id_with_eager, macro_id_to_def_id,
4242
nameres::{
4343
diagnostics::DefDiagnostic,
4444
mod_resolution::ModDir,
@@ -2187,7 +2187,7 @@ impl ModCollector<'_, '_> {
21872187
// scopes without eager expansion.
21882188

21892189
// Case 1: try to resolve macro calls with single-segment name and expand macro_rules
2190-
if let Ok(res) = macro_call_as_call_id(
2190+
if let Ok(res) = macro_call_as_call_id_with_eager(
21912191
db.upcast(),
21922192
&ast_id,
21932193
mac.expand_to,
@@ -2210,19 +2210,34 @@ impl ModCollector<'_, '_> {
22102210
.map(|it| macro_id_to_def_id(self.def_collector.db, it))
22112211
})
22122212
},
2213-
) {
2214-
// Legacy macros need to be expanded immediately, so that any macros they produce
2215-
// are in scope.
2216-
if let Some(val) = res {
2217-
self.def_collector.collect_macro_expansion(
2213+
|path| {
2214+
let resolved_res = self.def_collector.def_map.resolve_path_fp_with_macro(
2215+
db,
2216+
ResolveMode::Other,
22182217
self.module_id,
2219-
val,
2220-
self.macro_depth + 1,
2221-
container,
2218+
&path,
2219+
BuiltinShadowMode::Module,
2220+
Some(MacroSubNs::Bang),
22222221
);
2223-
}
2222+
resolved_res.resolved_def.take_macros().map(|it| macro_id_to_def_id(db, it))
2223+
},
2224+
) {
2225+
// FIXME: if there were errors, this mightve been in the eager expansion from an
2226+
// unresolved macro, so we need to push this into late macro resolution. see fixme above
2227+
if res.err.is_none() {
2228+
// Legacy macros need to be expanded immediately, so that any macros they produce
2229+
// are in scope.
2230+
if let Some(val) = res.value {
2231+
self.def_collector.collect_macro_expansion(
2232+
self.module_id,
2233+
val,
2234+
self.macro_depth + 1,
2235+
container,
2236+
);
2237+
}
22242238

2225-
return;
2239+
return;
2240+
}
22262241
}
22272242

22282243
// Case 2: resolve in module scope, expand during name resolution.

‎crates/hir-expand/src/eager.rs‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
//!
2020
//! See the full discussion : <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Eager.20expansion.20of.20built-in.20macros>
2121
use base_db::CrateId;
22-
use rustc_hash::FxHashMap;
22+
use rustc_hash::{FxHashMap, FxHashSet};
2323
use syntax::{ted, Parse, SyntaxNode, TextRange, TextSize, WalkEvent};
2424
use triomphe::Arc;
2525

@@ -29,7 +29,7 @@ use crate::{
2929
hygiene::Hygiene,
3030
mod_path::ModPath,
3131
EagerCallInfo, ExpandError, ExpandResult, ExpandTo, InFile, MacroCallId, MacroCallKind,
32-
MacroCallLoc, MacroDefId, MacroDefKind, UnresolvedMacro,
32+
MacroCallLoc, MacroDefId, MacroDefKind,
3333
};
3434

3535
pub fn expand_eager_macro_input(
@@ -38,7 +38,7 @@ pub fn expand_eager_macro_input(
3838
macro_call: InFile<ast::MacroCall>,
3939
def: MacroDefId,
4040
resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,
41-
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
41+
) -> ExpandResult<Option<MacroCallId>> {
4242
let ast_map = db.ast_id_map(macro_call.file_id);
4343
// 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!
4444
let call_id = InFile::new(macro_call.file_id, ast_map.ast_id(&macro_call.value));
@@ -71,22 +71,23 @@ pub fn expand_eager_macro_input(
7171
InFile::new(arg_id.as_file(), arg_exp.syntax_node()),
7272
krate,
7373
resolver,
74-
)?
74+
)
7575
};
7676
let err = parse_err.or(err);
7777

7878
let Some((expanded_eager_input, mapping)) = expanded_eager_input else {
79-
return Ok(ExpandResult { value: None, err });
79+
return ExpandResult { value: None, err };
8080
};
8181

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

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

9293
if let Some(range) = expanded_eager_input_token_map
@@ -97,13 +98,15 @@ pub fn expand_eager_macro_input(
9798
// remap from eager input expansion to original eager input
9899
if let Some(&og_range) = ws_mapping.get(og_range) {
99100
if let Some(og_token) = og_tmap.token_by_range(og_range) {
101+
ids_used.insert(og_token);
100102
return og_token;
101103
}
102104
}
103105
}
104106
}
105107
tt::TokenId::UNSPECIFIED
106108
});
109+
og_tmap.filter(|id| ids_used.contains(&id));
107110
og_tmap
108111
} else {
109112
Default::default()
@@ -121,7 +124,7 @@ pub fn expand_eager_macro_input(
121124
kind: MacroCallKind::FnLike { ast_id: call_id, expand_to },
122125
};
123126

124-
Ok(ExpandResult { value: Some(db.intern_macro_call(loc)), err })
127+
ExpandResult { value: Some(db.intern_macro_call(loc)), err }
125128
}
126129

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

154157
let mut replacements = Vec::new();
155158

156-
// Note: We only report a single error inside of eager expansions
159+
// FIXME: We only report a single error inside of eager expansions
157160
let mut error = None;
158161
let mut offset = 0i32;
159162
let apply_offset = |it: TextSize, offset: i32| {
@@ -184,24 +187,28 @@ fn eager_macro_recur(
184187
}
185188
};
186189
let def = match call.path().and_then(|path| ModPath::from_src(db, path, hygiene)) {
187-
Some(path) => macro_resolver(path.clone()).ok_or(UnresolvedMacro { path })?,
190+
Some(path) => match macro_resolver(path.clone()) {
191+
Some(def) => def,
192+
None => {
193+
error =
194+
Some(ExpandError::other(format!("unresolved macro {}", path.display(db))));
195+
continue;
196+
}
197+
},
188198
None => {
189199
error = Some(ExpandError::other("malformed macro invocation"));
190200
continue;
191201
}
192202
};
193203
let ExpandResult { value, err } = match def.kind {
194204
MacroDefKind::BuiltInEager(..) => {
195-
let ExpandResult { value, err } = match expand_eager_macro_input(
205+
let ExpandResult { value, err } = expand_eager_macro_input(
196206
db,
197207
krate,
198208
curr.with_value(call.clone()),
199209
def,
200210
macro_resolver,
201-
) {
202-
Ok(it) => it,
203-
Err(err) => return Err(err),
204-
};
211+
);
205212
match value {
206213
Some(call_id) => {
207214
let ExpandResult { value, err: err2 } =
@@ -251,7 +258,7 @@ fn eager_macro_recur(
251258
parse.as_ref().map(|it| it.syntax_node()),
252259
krate,
253260
macro_resolver,
254-
)?;
261+
);
255262
let err = err.or(error);
256263

257264
if let Some(tt) = call.token_tree() {
@@ -281,7 +288,7 @@ fn eager_macro_recur(
281288
}
282289
// check if the whole original syntax is replaced
283290
if call.syntax() == &original {
284-
return Ok(ExpandResult { value: value.zip(Some(mapping)), err: error });
291+
return ExpandResult { value: value.zip(Some(mapping)), err: error };
285292
}
286293

287294
if let Some(insert) = value {
@@ -292,5 +299,5 @@ fn eager_macro_recur(
292299
}
293300

294301
replacements.into_iter().rev().for_each(|(old, new)| ted::replace(old.syntax(), new));
295-
Ok(ExpandResult { value: Some((original, mapping)), err: error })
302+
ExpandResult { value: Some((original, mapping)), err: error }
296303
}

‎crates/ide-diagnostics/src/handlers/macro_error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ macro_rules! m {
8080
8181
fn f() {
8282
m!();
83-
//^^^^ error: unresolved macro `$crate::private::concat!`
83+
//^^^^ error: unresolved macro $crate::private::concat
8484
}
8585
8686
//- /core.rs crate:core

‎crates/ide/src/syntax_highlighting/test_data/highlight_macros.html‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,18 @@
9090
<span class="brace">}</span>
9191
<span class="brace">}</span>
9292

93+
<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>
94+
<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>
95+
<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>
96+
<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>
97+
<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>
98+
<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>
99+
100+
<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>
101+
93102
<span class="keyword">fn</span> <span class="function declaration">main</span><span class="parenthesis">(</span><span class="parenthesis">)</span> <span class="brace">{</span>
94-
<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>
103+
<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>
95104
<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>
96105
<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>
97-
<span class="brace">}</span></code></pre>
106+
<span class="brace">}</span>
107+
</code></pre>

0 commit comments

Comments
 (0)