Skip to content

Commit 32dce35

Browse files
committed
Auto merge of #82891 - cjgillot:monoparent, r=petrochenkov
Make def_key and HIR parenting consistent. r? `@petrochenkov`
2 parents 178bd91 + 445b4e3 commit 32dce35

File tree

11 files changed

+177
-54
lines changed

11 files changed

+177
-54
lines changed

compiler/rustc_ast_lowering/src/lib.rs

+28-21
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ enum ImplTraitContext<'b, 'a> {
223223
/// equivalent to a fresh universal parameter like `fn foo<T: Debug>(x: T)`.
224224
///
225225
/// Newly generated parameters should be inserted into the given `Vec`.
226-
Universal(&'b mut Vec<hir::GenericParam<'a>>),
226+
Universal(&'b mut Vec<hir::GenericParam<'a>>, LocalDefId),
227227

228228
/// Treat `impl Trait` as shorthand for a new opaque type.
229229
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
@@ -278,7 +278,7 @@ impl<'a> ImplTraitContext<'_, 'a> {
278278
fn reborrow<'this>(&'this mut self) -> ImplTraitContext<'this, 'a> {
279279
use self::ImplTraitContext::*;
280280
match self {
281-
Universal(params) => Universal(params),
281+
Universal(params, parent) => Universal(params, *parent),
282282
ReturnPositionOpaqueTy { fn_def_id, origin } => {
283283
ReturnPositionOpaqueTy { fn_def_id: *fn_def_id, origin: *origin }
284284
}
@@ -475,25 +475,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
475475
}
476476

477477
impl MiscCollector<'_, '_, '_> {
478-
fn allocate_use_tree_hir_id_counters(&mut self, tree: &UseTree, owner: LocalDefId) {
478+
fn allocate_use_tree_hir_id_counters(&mut self, tree: &UseTree) {
479479
match tree.kind {
480480
UseTreeKind::Simple(_, id1, id2) => {
481481
for &id in &[id1, id2] {
482-
self.lctx.resolver.create_def(
483-
owner,
484-
id,
485-
DefPathData::Misc,
486-
ExpnId::root(),
487-
tree.prefix.span,
488-
);
489482
self.lctx.allocate_hir_id_counter(id);
490483
}
491484
}
492485
UseTreeKind::Glob => (),
493486
UseTreeKind::Nested(ref trees) => {
494487
for &(ref use_tree, id) in trees {
495-
let hir_id = self.lctx.allocate_hir_id_counter(id);
496-
self.allocate_use_tree_hir_id_counters(use_tree, hir_id.owner);
488+
self.lctx.allocate_hir_id_counter(id);
489+
self.allocate_use_tree_hir_id_counters(use_tree);
497490
}
498491
}
499492
}
@@ -502,7 +495,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
502495

503496
impl<'tcx> Visitor<'tcx> for MiscCollector<'tcx, '_, '_> {
504497
fn visit_item(&mut self, item: &'tcx Item) {
505-
let hir_id = self.lctx.allocate_hir_id_counter(item.id);
498+
self.lctx.allocate_hir_id_counter(item.id);
506499

507500
match item.kind {
508501
ItemKind::Struct(_, ref generics)
@@ -521,7 +514,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
521514
self.lctx.type_def_lifetime_params.insert(def_id.to_def_id(), count);
522515
}
523516
ItemKind::Use(ref use_tree) => {
524-
self.allocate_use_tree_hir_id_counters(use_tree, hir_id.owner);
517+
self.allocate_use_tree_hir_id_counters(use_tree);
525518
}
526519
_ => {}
527520
}
@@ -939,8 +932,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
939932
// `lifetimes_to_define`. If we swapped the order of these two,
940933
// in-band-lifetimes introduced by generics or where-clauses
941934
// wouldn't have been added yet.
942-
let generics =
943-
this.lower_generics_mut(generics, ImplTraitContext::Universal(&mut params));
935+
let generics = this.lower_generics_mut(
936+
generics,
937+
ImplTraitContext::Universal(
938+
&mut params,
939+
this.current_hir_id_owner.last().unwrap().0,
940+
),
941+
);
944942
let res = f(this, &mut params);
945943
(params, (generics, res))
946944
})
@@ -1145,6 +1143,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
11451143
}
11461144
AssocTyConstraintKind::Bound { ref bounds } => {
11471145
let mut capturable_lifetimes;
1146+
let mut parent_def_id = self.current_hir_id_owner.last().unwrap().0;
11481147
// Piggy-back on the `impl Trait` context to figure out the correct behavior.
11491148
let (desugar_to_impl_trait, itctx) = match itctx {
11501149
// We are in the return position:
@@ -1164,7 +1163,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
11641163
// so desugar to
11651164
//
11661165
// fn foo(x: dyn Iterator<Item = impl Debug>)
1167-
ImplTraitContext::Universal(..) if self.is_in_dyn_type => (true, itctx),
1166+
ImplTraitContext::Universal(_, parent) if self.is_in_dyn_type => {
1167+
parent_def_id = parent;
1168+
(true, itctx)
1169+
}
11681170

11691171
// In `type Foo = dyn Iterator<Item: Debug>` we desugar to
11701172
// `type Foo = dyn Iterator<Item = impl Debug>` but we have to override the
@@ -1198,7 +1200,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
11981200
// constructing the HIR for `impl bounds...` and then lowering that.
11991201

12001202
let impl_trait_node_id = self.resolver.next_node_id();
1201-
let parent_def_id = self.current_hir_id_owner.last().unwrap().0;
12021203
self.resolver.create_def(
12031204
parent_def_id,
12041205
impl_trait_node_id,
@@ -1451,7 +1452,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
14511452
|this| this.lower_param_bounds(bounds, nested_itctx),
14521453
)
14531454
}
1454-
ImplTraitContext::Universal(in_band_ty_params) => {
1455+
ImplTraitContext::Universal(in_band_ty_params, parent_def_id) => {
14551456
// Add a definition for the in-band `Param`.
14561457
let def_id = self.resolver.local_def_id(def_node_id);
14571458

@@ -1460,7 +1461,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
14601461
let hir_bounds = self.with_hir_id_owner(def_node_id, |this| {
14611462
this.lower_param_bounds(
14621463
bounds,
1463-
ImplTraitContext::Universal(in_band_ty_params),
1464+
ImplTraitContext::Universal(in_band_ty_params, parent_def_id),
14641465
)
14651466
});
14661467
// Set the name to `impl Bound1 + Bound2`.
@@ -1891,7 +1892,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
18911892
}
18921893
this.arena.alloc_from_iter(inputs.iter().map(|param| {
18931894
if let Some((_, ibty)) = &mut in_band_ty_params {
1894-
this.lower_ty_direct(&param.ty, ImplTraitContext::Universal(ibty))
1895+
this.lower_ty_direct(
1896+
&param.ty,
1897+
ImplTraitContext::Universal(
1898+
ibty,
1899+
this.current_hir_id_owner.last().unwrap().0,
1900+
),
1901+
)
18951902
} else {
18961903
this.lower_ty_direct(&param.ty, ImplTraitContext::disallowed())
18971904
}

compiler/rustc_middle/src/hir/map/collector.rs

+12
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,18 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
221221

222222
data.signature =
223223
Some(self.arena.alloc(Owner { parent: entry.parent, node: entry.node }));
224+
225+
let dk_parent = self.definitions.def_key(id.owner).parent;
226+
if let Some(dk_parent) = dk_parent {
227+
let dk_parent = LocalDefId { local_def_index: dk_parent };
228+
let dk_parent = self.definitions.local_def_id_to_hir_id(dk_parent);
229+
if dk_parent.owner != entry.parent.owner {
230+
panic!(
231+
"Different parents for {:?} => dk_parent={:?} actual={:?}",
232+
id.owner, dk_parent, entry.parent,
233+
)
234+
}
235+
}
224236
} else {
225237
assert_eq!(entry.parent.owner, id.owner);
226238
insert_vec_map(

compiler/rustc_resolve/src/def_collector.rs

+71-18
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::Resolver;
1+
use crate::{ImplTraitContext, Resolver};
22
use rustc_ast::visit::{self, FnKind};
33
use rustc_ast::walk_list;
44
use rustc_ast::*;
@@ -16,14 +16,15 @@ crate fn collect_definitions(
1616
fragment: &AstFragment,
1717
expansion: ExpnId,
1818
) {
19-
let parent_def = resolver.invocation_parents[&expansion];
20-
fragment.visit_with(&mut DefCollector { resolver, parent_def, expansion });
19+
let (parent_def, impl_trait_context) = resolver.invocation_parents[&expansion];
20+
fragment.visit_with(&mut DefCollector { resolver, parent_def, expansion, impl_trait_context });
2121
}
2222

2323
/// Creates `DefId`s for nodes in the AST.
2424
struct DefCollector<'a, 'b> {
2525
resolver: &'a mut Resolver<'b>,
2626
parent_def: LocalDefId,
27+
impl_trait_context: ImplTraitContext,
2728
expansion: ExpnId,
2829
}
2930

@@ -40,6 +41,16 @@ impl<'a, 'b> DefCollector<'a, 'b> {
4041
self.parent_def = orig_parent_def;
4142
}
4243

44+
fn with_impl_trait<F: FnOnce(&mut Self)>(
45+
&mut self,
46+
impl_trait_context: ImplTraitContext,
47+
f: F,
48+
) {
49+
let orig_itc = std::mem::replace(&mut self.impl_trait_context, impl_trait_context);
50+
f(self);
51+
self.impl_trait_context = orig_itc;
52+
}
53+
4354
fn collect_field(&mut self, field: &'a StructField, index: Option<usize>) {
4455
let index = |this: &Self| {
4556
index.unwrap_or_else(|| {
@@ -60,8 +71,9 @@ impl<'a, 'b> DefCollector<'a, 'b> {
6071
}
6172

6273
fn visit_macro_invoc(&mut self, id: NodeId) {
74+
let id = id.placeholder_to_expn_id();
6375
let old_parent =
64-
self.resolver.invocation_parents.insert(id.placeholder_to_expn_id(), self.parent_def);
76+
self.resolver.invocation_parents.insert(id, (self.parent_def, self.impl_trait_context));
6577
assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation");
6678
}
6779
}
@@ -103,29 +115,37 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
103115
let def = self.create_def(i.id, def_data, i.span);
104116

105117
self.with_parent(def, |this| {
106-
match i.kind {
107-
ItemKind::Struct(ref struct_def, _) | ItemKind::Union(ref struct_def, _) => {
108-
// If this is a unit or tuple-like struct, register the constructor.
109-
if let Some(ctor_hir_id) = struct_def.ctor_id() {
110-
this.create_def(ctor_hir_id, DefPathData::Ctor, i.span);
118+
this.with_impl_trait(ImplTraitContext::Existential, |this| {
119+
match i.kind {
120+
ItemKind::Struct(ref struct_def, _) | ItemKind::Union(ref struct_def, _) => {
121+
// If this is a unit or tuple-like struct, register the constructor.
122+
if let Some(ctor_hir_id) = struct_def.ctor_id() {
123+
this.create_def(ctor_hir_id, DefPathData::Ctor, i.span);
124+
}
111125
}
126+
_ => {}
112127
}
113-
_ => {}
114-
}
115-
visit::walk_item(this, i);
128+
visit::walk_item(this, i);
129+
})
116130
});
117131
}
118132

119133
fn visit_fn(&mut self, fn_kind: FnKind<'a>, span: Span, _: NodeId) {
120134
if let FnKind::Fn(_, _, sig, _, body) = fn_kind {
121135
if let Async::Yes { closure_id, return_impl_trait_id, .. } = sig.header.asyncness {
122-
self.create_def(return_impl_trait_id, DefPathData::ImplTrait, span);
136+
let return_impl_trait_id =
137+
self.create_def(return_impl_trait_id, DefPathData::ImplTrait, span);
123138

124139
// For async functions, we need to create their inner defs inside of a
125140
// closure to match their desugared representation. Besides that,
126141
// we must mirror everything that `visit::walk_fn` below does.
127142
self.visit_fn_header(&sig.header);
128-
visit::walk_fn_decl(self, &sig.decl);
143+
for param in &sig.decl.inputs {
144+
self.visit_param(param);
145+
}
146+
self.with_parent(return_impl_trait_id, |this| {
147+
this.visit_fn_ret_ty(&sig.decl.output)
148+
});
129149
let closure_def = self.create_def(closure_id, DefPathData::ClosureExpr, span);
130150
self.with_parent(closure_def, |this| walk_list!(this, visit_block, body));
131151
return;
@@ -137,6 +157,14 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
137157

138158
fn visit_use_tree(&mut self, use_tree: &'a UseTree, id: NodeId, _nested: bool) {
139159
self.create_def(id, DefPathData::Misc, use_tree.span);
160+
match use_tree.kind {
161+
UseTreeKind::Simple(_, id1, id2) => {
162+
self.create_def(id1, DefPathData::Misc, use_tree.prefix.span);
163+
self.create_def(id2, DefPathData::Misc, use_tree.prefix.span);
164+
}
165+
UseTreeKind::Glob => (),
166+
UseTreeKind::Nested(..) => {}
167+
}
140168
visit::walk_use_tree(self, use_tree, id);
141169
}
142170

@@ -191,7 +219,15 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
191219
};
192220
self.create_def(param.id, def_path_data, param.ident.span);
193221

194-
visit::walk_generic_param(self, param);
222+
// impl-Trait can happen inside generic parameters, like
223+
// ```
224+
// fn foo<U: Iterator<Item = impl Clone>>() {}
225+
// ```
226+
//
227+
// In that case, the impl-trait is lowered as an additional generic parameter.
228+
self.with_impl_trait(ImplTraitContext::Universal(self.parent_def), |this| {
229+
visit::walk_generic_param(this, param)
230+
});
195231
}
196232

197233
fn visit_assoc_item(&mut self, i: &'a AssocItem, ctxt: visit::AssocCtxt) {
@@ -244,8 +280,19 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
244280
match ty.kind {
245281
TyKind::MacCall(..) => self.visit_macro_invoc(ty.id),
246282
TyKind::ImplTrait(node_id, _) => {
247-
let parent_def = self.create_def(node_id, DefPathData::ImplTrait, ty.span);
248-
self.with_parent(parent_def, |this| visit::walk_ty(this, ty));
283+
let parent_def = match self.impl_trait_context {
284+
ImplTraitContext::Universal(item_def) => self.resolver.create_def(
285+
item_def,
286+
node_id,
287+
DefPathData::ImplTrait,
288+
self.expansion,
289+
ty.span,
290+
),
291+
ImplTraitContext::Existential => {
292+
self.create_def(node_id, DefPathData::ImplTrait, ty.span)
293+
}
294+
};
295+
self.with_parent(parent_def, |this| visit::walk_ty(this, ty))
249296
}
250297
_ => visit::walk_ty(self, ty),
251298
}
@@ -275,7 +322,13 @@ impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> {
275322
}
276323

277324
fn visit_param(&mut self, p: &'a Param) {
278-
if p.is_placeholder { self.visit_macro_invoc(p.id) } else { visit::walk_param(self, p) }
325+
if p.is_placeholder {
326+
self.visit_macro_invoc(p.id)
327+
} else {
328+
self.with_impl_trait(ImplTraitContext::Universal(self.parent_def), |this| {
329+
visit::walk_param(this, p)
330+
})
331+
}
279332
}
280333

281334
// This method is called only when we are visiting an individual field

compiler/rustc_resolve/src/lib.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ impl<'a> ParentScope<'a> {
156156
}
157157
}
158158

159+
#[derive(Copy, Debug, Clone)]
160+
enum ImplTraitContext {
161+
Existential,
162+
Universal(LocalDefId),
163+
}
164+
159165
#[derive(Eq)]
160166
struct BindingError {
161167
name: Symbol,
@@ -989,8 +995,9 @@ pub struct Resolver<'a> {
989995
/// Indices of unnamed struct or variant fields with unresolved attributes.
990996
placeholder_field_indices: FxHashMap<NodeId, usize>,
991997
/// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId`
992-
/// we know what parent node that fragment should be attached to thanks to this table.
993-
invocation_parents: FxHashMap<ExpnId, LocalDefId>,
998+
/// we know what parent node that fragment should be attached to thanks to this table,
999+
/// and how the `impl Trait` fragments were introduced.
1000+
invocation_parents: FxHashMap<ExpnId, (LocalDefId, ImplTraitContext)>,
9941001

9951002
next_disambiguator: FxHashMap<(LocalDefId, DefPathData), u32>,
9961003
/// Some way to know that we are in a *trait* impl in `visit_assoc_item`.
@@ -1205,7 +1212,7 @@ impl<'a> Resolver<'a> {
12051212
node_id_to_def_id.insert(CRATE_NODE_ID, root);
12061213

12071214
let mut invocation_parents = FxHashMap::default();
1208-
invocation_parents.insert(ExpnId::root(), root);
1215+
invocation_parents.insert(ExpnId::root(), (root, ImplTraitContext::Existential));
12091216

12101217
let mut extern_prelude: FxHashMap<Ident, ExternPreludeEntry<'_>> = session
12111218
.opts

compiler/rustc_resolve/src/macros.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ impl<'a> ResolverExpand for Resolver<'a> {
326326
// nearest closing item - we should try to return the closest parent of the ExpnId
327327
self.invocation_parents
328328
.get(&expn_id)
329-
.map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id])
329+
.map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[id.0])
330330
}
331331

332332
fn has_derive_copy(&self, expn_id: ExpnId) -> bool {

src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt

-3
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@
9393
59| 1| used_only_from_this_lib_crate_generic_function(some_vec);
9494
60| 1| used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs");
9595
61| 1|}
96-
------------------
97-
| Unexecuted instantiation: used_crate::use_this_lib_crate
98-
------------------
9996
62| |
10097
63| |// FIXME(#79651): `used_from_bin_crate_and_lib_crate_generic_function()` is covered and executed
10198
64| |// `2` times, but the coverage output also shows (at the bottom of the coverage report):

src/test/ui/consts/miri_unleashed/tls.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ error[E0080]: could not evaluate static initializer
22
--> $DIR/tls.rs:12:25
33
|
44
LL | unsafe { let _val = A; }
5-
| ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A))
5+
| ^ cannot access thread local static (DefId(0:6 ~ tls[317d]::A))
66

77
error[E0080]: could not evaluate static initializer
88
--> $DIR/tls.rs:19:26
99
|
1010
LL | unsafe { let _val = &A; }
11-
| ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A))
11+
| ^ cannot access thread local static (DefId(0:6 ~ tls[317d]::A))
1212

1313
warning: skipping const checks
1414
|

0 commit comments

Comments
 (0)