Skip to content

Commit 4dba1b4

Browse files
ShoyuVanillaShoyuVanilla
authored
Merge pull request #21633 from ChayimFriedman2/proc-macro-is-not-fn
fix: Do not resolve proc macros in value ns (as functions), only in macro ns, outside their defining crate
2 parents 50863f4 + 23a19cd commit 4dba1b4

15 files changed

Lines changed: 120 additions & 60 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,11 @@ impl ItemScope {
483483
self.declarations.push(def)
484484
}
485485

486+
pub(crate) fn remove_from_value_ns(&mut self, name: &Name, def: ModuleDefId) {
487+
let entry = self.values.shift_remove(name);
488+
assert!(entry.is_some_and(|entry| entry.def == def))
489+
}
490+
486491
pub(crate) fn get_legacy_macro(&self, name: &Name) -> Option<&[MacroId]> {
487492
self.legacy_macros.get(name).map(|it| &**it)
488493
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ struct DefMapCrateData {
211211
/// Side table for resolving derive helpers.
212212
exported_derives: FxHashMap<MacroId, Box<[Name]>>,
213213
fn_proc_macro_mapping: FxHashMap<FunctionId, ProcMacroId>,
214+
fn_proc_macro_mapping_back: FxHashMap<ProcMacroId, FunctionId>,
214215

215216
/// Custom tool modules registered with `#![register_tool]`.
216217
registered_tools: Vec<Symbol>,
@@ -230,6 +231,7 @@ impl DefMapCrateData {
230231
Self {
231232
exported_derives: FxHashMap::default(),
232233
fn_proc_macro_mapping: FxHashMap::default(),
234+
fn_proc_macro_mapping_back: FxHashMap::default(),
233235
registered_tools: PREDEFINED_TOOLS.iter().map(|it| Symbol::intern(it)).collect(),
234236
unstable_features: FxHashSet::default(),
235237
rustc_coherence_is_core: false,
@@ -244,6 +246,7 @@ impl DefMapCrateData {
244246
let Self {
245247
exported_derives,
246248
fn_proc_macro_mapping,
249+
fn_proc_macro_mapping_back,
247250
registered_tools,
248251
unstable_features,
249252
rustc_coherence_is_core: _,
@@ -254,6 +257,7 @@ impl DefMapCrateData {
254257
} = self;
255258
exported_derives.shrink_to_fit();
256259
fn_proc_macro_mapping.shrink_to_fit();
260+
fn_proc_macro_mapping_back.shrink_to_fit();
257261
registered_tools.shrink_to_fit();
258262
unstable_features.shrink_to_fit();
259263
}
@@ -570,6 +574,10 @@ impl DefMap {
570574
self.data.fn_proc_macro_mapping.get(&id).copied()
571575
}
572576

577+
pub fn proc_macro_as_fn(&self, id: ProcMacroId) -> Option<FunctionId> {
578+
self.data.fn_proc_macro_mapping_back.get(&id).copied()
579+
}
580+
573581
pub fn krate(&self) -> Crate {
574582
self.krate
575583
}

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,7 @@ impl<'db> DefCollector<'db> {
634634
crate_data.exported_derives.insert(proc_macro_id.into(), helpers);
635635
}
636636
crate_data.fn_proc_macro_mapping.insert(fn_id, proc_macro_id);
637+
crate_data.fn_proc_macro_mapping_back.insert(proc_macro_id, fn_id);
637638
}
638639

639640
/// Define a macro with `macro_rules`.
@@ -2095,6 +2096,8 @@ impl ModCollector<'_, '_> {
20952096

20962097
let vis = resolve_vis(def_map, local_def_map, &self.item_tree[it.visibility]);
20972098

2099+
update_def(self.def_collector, fn_id.into(), &it.name, vis, false);
2100+
20982101
if self.def_collector.def_map.block.is_none()
20992102
&& self.def_collector.is_proc_macro
21002103
&& self.module_id == self.def_collector.def_map.root
@@ -2105,9 +2108,14 @@ impl ModCollector<'_, '_> {
21052108
InFile::new(self.file_id(), id),
21062109
fn_id,
21072110
);
2108-
}
21092111

2110-
update_def(self.def_collector, fn_id.into(), &it.name, vis, false);
2112+
// A proc macro is implemented as a function, but it's treated as a macro, not a function.
2113+
// You cannot call it like a function, for example, except in its defining crate.
2114+
// So we keep the function definition, but remove it from the scope, leaving only the macro.
2115+
self.def_collector.def_map[module_id]
2116+
.scope
2117+
.remove_from_value_ns(&it.name, fn_id.into());
2118+
}
21112119
}
21122120
ModItemId::Struct(id) => {
21132121
let it = &self.item_tree[id];

‎crates/hir-def/src/nameres/tests/macros.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,10 +1068,8 @@ pub fn derive_macro_2(_item: TokenStream) -> TokenStream {
10681068
- AnotherTrait : macro#
10691069
- DummyTrait : macro#
10701070
- TokenStream : type value
1071-
- attribute_macro : value macro#
1072-
- derive_macro : value
1073-
- derive_macro_2 : value
1074-
- function_like_macro : value macro!
1071+
- attribute_macro : macro#
1072+
- function_like_macro : macro!
10751073
"#]],
10761074
);
10771075
}

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

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::{
3232
BindingId, ExprId, LabelId,
3333
generics::{GenericParams, TypeOrConstParamData},
3434
},
35-
item_scope::{BUILTIN_SCOPE, BuiltinShadowMode, ImportOrExternCrate, ImportOrGlob, ItemScope},
35+
item_scope::{BUILTIN_SCOPE, BuiltinShadowMode, ImportOrExternCrate, ItemScope},
3636
lang_item::LangItemTarget,
3737
nameres::{DefMap, LocalDefMap, MacroSubNs, ResolvePathResultPrefixInfo, block_def_map},
3838
per_ns::PerNs,
@@ -111,8 +111,8 @@ pub enum TypeNs {
111111

112112
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
113113
pub enum ResolveValueResult {
114-
ValueNs(ValueNs, Option<ImportOrGlob>),
115-
Partial(TypeNs, usize, Option<ImportOrExternCrate>),
114+
ValueNs(ValueNs),
115+
Partial(TypeNs, usize),
116116
}
117117

118118
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -332,20 +332,17 @@ impl<'db> Resolver<'db> {
332332
Path::Normal(it) => &it.mod_path,
333333
Path::LangItem(l, None) => {
334334
return Some((
335-
ResolveValueResult::ValueNs(
336-
match *l {
337-
LangItemTarget::FunctionId(it) => ValueNs::FunctionId(it),
338-
LangItemTarget::StaticId(it) => ValueNs::StaticId(it),
339-
LangItemTarget::StructId(it) => ValueNs::StructId(it),
340-
LangItemTarget::EnumVariantId(it) => ValueNs::EnumVariantId(it),
341-
LangItemTarget::UnionId(_)
342-
| LangItemTarget::ImplId(_)
343-
| LangItemTarget::TypeAliasId(_)
344-
| LangItemTarget::TraitId(_)
345-
| LangItemTarget::EnumId(_) => return None,
346-
},
347-
None,
348-
),
335+
ResolveValueResult::ValueNs(match *l {
336+
LangItemTarget::FunctionId(it) => ValueNs::FunctionId(it),
337+
LangItemTarget::StaticId(it) => ValueNs::StaticId(it),
338+
LangItemTarget::StructId(it) => ValueNs::StructId(it),
339+
LangItemTarget::EnumVariantId(it) => ValueNs::EnumVariantId(it),
340+
LangItemTarget::UnionId(_)
341+
| LangItemTarget::ImplId(_)
342+
| LangItemTarget::TypeAliasId(_)
343+
| LangItemTarget::TraitId(_)
344+
| LangItemTarget::EnumId(_) => return None,
345+
}),
349346
ResolvePathResultPrefixInfo::default(),
350347
));
351348
}
@@ -363,7 +360,7 @@ impl<'db> Resolver<'db> {
363360
};
364361
// Remaining segments start from 0 because lang paths have no segments other than the remaining.
365362
return Some((
366-
ResolveValueResult::Partial(type_ns, 0, None),
363+
ResolveValueResult::Partial(type_ns, 0),
367364
ResolvePathResultPrefixInfo::default(),
368365
));
369366
}
@@ -388,10 +385,7 @@ impl<'db> Resolver<'db> {
388385

389386
if let Some(e) = entry {
390387
return Some((
391-
ResolveValueResult::ValueNs(
392-
ValueNs::LocalBinding(e.binding()),
393-
None,
394-
),
388+
ResolveValueResult::ValueNs(ValueNs::LocalBinding(e.binding())),
395389
ResolvePathResultPrefixInfo::default(),
396390
));
397391
}
@@ -404,14 +398,14 @@ impl<'db> Resolver<'db> {
404398
&& *first_name == sym::Self_
405399
{
406400
return Some((
407-
ResolveValueResult::ValueNs(ValueNs::ImplSelf(impl_), None),
401+
ResolveValueResult::ValueNs(ValueNs::ImplSelf(impl_)),
408402
ResolvePathResultPrefixInfo::default(),
409403
));
410404
}
411405
if let Some(id) = params.find_const_by_name(first_name, *def) {
412406
let val = ValueNs::GenericParam(id);
413407
return Some((
414-
ResolveValueResult::ValueNs(val, None),
408+
ResolveValueResult::ValueNs(val),
415409
ResolvePathResultPrefixInfo::default(),
416410
));
417411
}
@@ -431,7 +425,7 @@ impl<'db> Resolver<'db> {
431425
if let &GenericDefId::ImplId(impl_) = def {
432426
if *first_name == sym::Self_ {
433427
return Some((
434-
ResolveValueResult::Partial(TypeNs::SelfType(impl_), 1, None),
428+
ResolveValueResult::Partial(TypeNs::SelfType(impl_), 1),
435429
ResolvePathResultPrefixInfo::default(),
436430
));
437431
}
@@ -440,14 +434,14 @@ impl<'db> Resolver<'db> {
440434
{
441435
let ty = TypeNs::AdtSelfType(adt);
442436
return Some((
443-
ResolveValueResult::Partial(ty, 1, None),
437+
ResolveValueResult::Partial(ty, 1),
444438
ResolvePathResultPrefixInfo::default(),
445439
));
446440
}
447441
if let Some(id) = params.find_type_by_name(first_name, *def) {
448442
let ty = TypeNs::GenericParam(id);
449443
return Some((
450-
ResolveValueResult::Partial(ty, 1, None),
444+
ResolveValueResult::Partial(ty, 1),
451445
ResolvePathResultPrefixInfo::default(),
452446
));
453447
}
@@ -473,7 +467,7 @@ impl<'db> Resolver<'db> {
473467
&& let Some(builtin) = BuiltinType::by_name(first_name)
474468
{
475469
return Some((
476-
ResolveValueResult::Partial(TypeNs::BuiltinType(builtin), 1, None),
470+
ResolveValueResult::Partial(TypeNs::BuiltinType(builtin), 1),
477471
ResolvePathResultPrefixInfo::default(),
478472
));
479473
}
@@ -488,7 +482,7 @@ impl<'db> Resolver<'db> {
488482
hygiene: HygieneId,
489483
) -> Option<ValueNs> {
490484
match self.resolve_path_in_value_ns(db, path, hygiene)? {
491-
ResolveValueResult::ValueNs(it, _) => Some(it),
485+
ResolveValueResult::ValueNs(it) => Some(it),
492486
ResolveValueResult::Partial(..) => None,
493487
}
494488
}
@@ -1153,12 +1147,12 @@ impl<'db> ModuleItemMap<'db> {
11531147
);
11541148
match unresolved_idx {
11551149
None => {
1156-
let (value, import) = to_value_ns(module_def)?;
1157-
Some((ResolveValueResult::ValueNs(value, import), prefix_info))
1150+
let value = to_value_ns(module_def, self.def_map)?;
1151+
Some((ResolveValueResult::ValueNs(value), prefix_info))
11581152
}
11591153
Some(unresolved_idx) => {
1160-
let def = module_def.take_types_full()?;
1161-
let ty = match def.def {
1154+
let def = module_def.take_types()?;
1155+
let ty = match def {
11621156
ModuleDefId::AdtId(it) => TypeNs::AdtId(it),
11631157
ModuleDefId::TraitId(it) => TypeNs::TraitId(it),
11641158
ModuleDefId::TypeAliasId(it) => TypeNs::TypeAliasId(it),
@@ -1171,7 +1165,7 @@ impl<'db> ModuleItemMap<'db> {
11711165
| ModuleDefId::MacroId(_)
11721166
| ModuleDefId::StaticId(_) => return None,
11731167
};
1174-
Some((ResolveValueResult::Partial(ty, unresolved_idx, def.import), prefix_info))
1168+
Some((ResolveValueResult::Partial(ty, unresolved_idx), prefix_info))
11751169
}
11761170
}
11771171
}
@@ -1194,8 +1188,13 @@ impl<'db> ModuleItemMap<'db> {
11941188
}
11951189
}
11961190

1197-
fn to_value_ns(per_ns: PerNs) -> Option<(ValueNs, Option<ImportOrGlob>)> {
1198-
let (def, import) = per_ns.take_values_import()?;
1191+
fn to_value_ns(per_ns: PerNs, def_map: &DefMap) -> Option<ValueNs> {
1192+
let def = per_ns.take_values().or_else(|| {
1193+
let Some(MacroId::ProcMacroId(proc_macro)) = per_ns.take_macros() else { return None };
1194+
// If we cannot resolve to value ns, but we can resolve to a proc macro, and this is the crate
1195+
// defining this proc macro - inside this crate, we should treat the macro as a function.
1196+
def_map.proc_macro_as_fn(proc_macro).map(ModuleDefId::FunctionId)
1197+
})?;
11991198
let res = match def {
12001199
ModuleDefId::FunctionId(it) => ValueNs::FunctionId(it),
12011200
ModuleDefId::AdtId(AdtId::StructId(it)) => ValueNs::StructId(it),
@@ -1210,7 +1209,7 @@ fn to_value_ns(per_ns: PerNs) -> Option<(ValueNs, Option<ImportOrGlob>)> {
12101209
| ModuleDefId::MacroId(_)
12111210
| ModuleDefId::ModuleId(_) => return None,
12121211
};
1213-
Some((res, import))
1212+
Some(res)
12141213
}
12151214

12161215
fn to_type_ns(per_ns: PerNs) -> Option<(TypeNs, Option<ImportOrExternCrate>)> {

‎crates/hir-ty/src/diagnostics/unsafe_check.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ impl<'db> UnsafeVisitor<'db> {
430430
fn mark_unsafe_path(&mut self, node: ExprOrPatId, path: &Path) {
431431
let hygiene = self.body.expr_or_pat_path_hygiene(node);
432432
let value_or_partial = self.resolver.resolve_path_in_value_ns(self.db, path, hygiene);
433-
if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial {
433+
if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id))) = value_or_partial {
434434
let static_data = self.db.static_signature(id);
435435
if static_data.flags.contains(StaticFlags::MUTABLE) {
436436
self.on_unsafe_op(node, UnsafetyReason::MutableStatic);

‎crates/hir-ty/src/infer.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,7 +1584,7 @@ impl<'body, 'db> InferenceContext<'body, 'db> {
15841584
return (self.err_ty(), None);
15851585
};
15861586
match res {
1587-
ResolveValueResult::ValueNs(value, _) => match value {
1587+
ResolveValueResult::ValueNs(value) => match value {
15881588
ValueNs::EnumVariantId(var) => {
15891589
let args = path_ctx.substs_from_path(var.into(), true, false);
15901590
drop(ctx);
@@ -1608,7 +1608,7 @@ impl<'body, 'db> InferenceContext<'body, 'db> {
16081608
return (self.err_ty(), None);
16091609
}
16101610
},
1611-
ResolveValueResult::Partial(typens, unresolved, _) => (typens, Some(unresolved)),
1611+
ResolveValueResult::Partial(typens, unresolved) => (typens, Some(unresolved)),
16121612
}
16131613
} else {
16141614
match path_ctx.resolve_path_in_type_ns() {

‎crates/hir-ty/src/infer/path.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,11 @@ impl<'db> InferenceContext<'_, 'db> {
164164
let value_or_partial = path_ctx.resolve_path_in_value_ns(hygiene)?;
165165

166166
match value_or_partial {
167-
ResolveValueResult::ValueNs(it, _) => {
167+
ResolveValueResult::ValueNs(it) => {
168168
drop_ctx(ctx, no_diagnostics);
169169
(it, None)
170170
}
171-
ResolveValueResult::Partial(def, remaining_index, _) => {
171+
ResolveValueResult::Partial(def, remaining_index) => {
172172
// there may be more intermediate segments between the resolved one and
173173
// the end. Only the last segment needs to be resolved to a value; from
174174
// the segments before that, we need to get either a type or a trait ref.

‎crates/hir-ty/src/lower/path.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -396,12 +396,10 @@ impl<'a, 'b, 'db> PathLoweringContext<'a, 'b, 'db> {
396396
}
397397

398398
let (mod_segments, enum_segment, resolved_segment_idx) = match res {
399-
ResolveValueResult::Partial(_, unresolved_segment, _) => {
399+
ResolveValueResult::Partial(_, unresolved_segment) => {
400400
(segments.take(unresolved_segment - 1), None, unresolved_segment - 1)
401401
}
402-
ResolveValueResult::ValueNs(ValueNs::EnumVariantId(_), _)
403-
if prefix_info.enum_variant =>
404-
{
402+
ResolveValueResult::ValueNs(ValueNs::EnumVariantId(_)) if prefix_info.enum_variant => {
405403
(segments.strip_last_two(), segments.len().checked_sub(2), segments.len() - 1)
406404
}
407405
ResolveValueResult::ValueNs(..) => (segments.strip_last(), None, segments.len() - 1),
@@ -431,7 +429,7 @@ impl<'a, 'b, 'db> PathLoweringContext<'a, 'b, 'db> {
431429
}
432430

433431
match &res {
434-
ResolveValueResult::ValueNs(resolution, _) => {
432+
ResolveValueResult::ValueNs(resolution) => {
435433
let resolved_segment_idx = self.current_segment_u32();
436434
let resolved_segment = self.current_or_prev_segment;
437435

@@ -469,7 +467,7 @@ impl<'a, 'b, 'db> PathLoweringContext<'a, 'b, 'db> {
469467
| ValueNs::ConstId(_) => {}
470468
}
471469
}
472-
ResolveValueResult::Partial(resolution, _, _) => {
470+
ResolveValueResult::Partial(resolution, _) => {
473471
if !self.handle_type_ns_resolution(resolution) {
474472
return None;
475473
}

‎crates/hir-ty/src/mir/lower.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,7 +1429,7 @@ impl<'a, 'db> MirLowerCtx<'a, 'db> {
14291429
.resolve_path_in_value_ns(self.db, c, HygieneId::ROOT)
14301430
.ok_or_else(unresolved_name)?;
14311431
match pr {
1432-
ResolveValueResult::ValueNs(v, _) => {
1432+
ResolveValueResult::ValueNs(v) => {
14331433
if let ValueNs::ConstId(c) = v {
14341434
self.lower_const_to_operand(
14351435
GenericArgs::empty(self.interner()),
@@ -1439,7 +1439,7 @@ impl<'a, 'db> MirLowerCtx<'a, 'db> {
14391439
not_supported!("bad path in range pattern");
14401440
}
14411441
}
1442-
ResolveValueResult::Partial(_, _, _) => {
1442+
ResolveValueResult::Partial(_, _) => {
14431443
not_supported!("associated constants in range pattern")
14441444
}
14451445
}

0 commit comments

Comments
 (0)