Skip to content

Commit baf382e

Browse files
committed
Auto merge of #98396 - cjgillot:iwfchir, r=petrochenkov
Do not access HIR to check impl wf. r? `@ghost`
2 parents 64eb9ab + 8242aa8 commit baf382e

File tree

8 files changed

+49
-74
lines changed

8 files changed

+49
-74
lines changed

Diff for: compiler/rustc_typeck/src/impl_wf_check.rs

+19-28
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use min_specialization::check_min_specialization;
1313

1414
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1515
use rustc_errors::struct_span_err;
16-
use rustc_hir as hir;
1716
use rustc_hir::def::DefKind;
1817
use rustc_hir::def_id::LocalDefId;
1918
use rustc_middle::ty::query::Providers;
@@ -59,13 +58,10 @@ fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
5958
let module = tcx.hir_module_items(module_def_id);
6059
for id in module.items() {
6160
if matches!(tcx.def_kind(id.def_id), DefKind::Impl) {
62-
let item = tcx.hir().item(id);
63-
if let hir::ItemKind::Impl(ref impl_) = item.kind {
64-
enforce_impl_params_are_constrained(tcx, item.def_id, impl_.items);
65-
enforce_impl_items_are_distinct(tcx, impl_.items);
66-
if min_specialization {
67-
check_min_specialization(tcx, item.def_id.to_def_id(), item.span);
68-
}
61+
enforce_impl_params_are_constrained(tcx, id.def_id);
62+
enforce_impl_items_are_distinct(tcx, id.def_id);
63+
if min_specialization {
64+
check_min_specialization(tcx, id.def_id);
6965
}
7066
}
7167
}
@@ -75,11 +71,7 @@ pub fn provide(providers: &mut Providers) {
7571
*providers = Providers { check_mod_impl_wf, ..*providers };
7672
}
7773

78-
fn enforce_impl_params_are_constrained(
79-
tcx: TyCtxt<'_>,
80-
impl_def_id: LocalDefId,
81-
impl_item_refs: &[hir::ImplItemRef],
82-
) {
74+
fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
8375
// Every lifetime used in an associated type must be constrained.
8476
let impl_self_ty = tcx.type_of(impl_def_id);
8577
if impl_self_ty.references_error() {
@@ -107,9 +99,9 @@ fn enforce_impl_params_are_constrained(
10799
);
108100

109101
// Disallow unconstrained lifetimes, but only if they appear in assoc types.
110-
let lifetimes_in_associated_types: FxHashSet<_> = impl_item_refs
102+
let lifetimes_in_associated_types: FxHashSet<_> = tcx
103+
.associated_item_def_ids(impl_def_id)
111104
.iter()
112-
.map(|item_ref| item_ref.id.def_id)
113105
.flat_map(|def_id| {
114106
let item = tcx.associated_item(def_id);
115107
match item.kind {
@@ -209,33 +201,32 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: &str)
209201
}
210202

211203
/// Enforce that we do not have two items in an impl with the same name.
212-
fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef]) {
204+
fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
213205
let mut seen_type_items = FxHashMap::default();
214206
let mut seen_value_items = FxHashMap::default();
215-
for impl_item_ref in impl_item_refs {
216-
let impl_item = tcx.hir().impl_item(impl_item_ref.id);
207+
for &impl_item_ref in tcx.associated_item_def_ids(impl_def_id) {
208+
let impl_item = tcx.associated_item(impl_item_ref);
217209
let seen_items = match impl_item.kind {
218-
hir::ImplItemKind::TyAlias(_) => &mut seen_type_items,
210+
ty::AssocKind::Type => &mut seen_type_items,
219211
_ => &mut seen_value_items,
220212
};
221-
match seen_items.entry(impl_item.ident.normalize_to_macros_2_0()) {
213+
let span = tcx.def_span(impl_item_ref);
214+
let ident = impl_item.ident(tcx);
215+
match seen_items.entry(ident.normalize_to_macros_2_0()) {
222216
Occupied(entry) => {
223217
let mut err = struct_span_err!(
224218
tcx.sess,
225-
impl_item.span,
219+
span,
226220
E0201,
227221
"duplicate definitions with name `{}`:",
228-
impl_item.ident
229-
);
230-
err.span_label(
231-
*entry.get(),
232-
format!("previous definition of `{}` here", impl_item.ident),
222+
ident
233223
);
234-
err.span_label(impl_item.span, "duplicate definition");
224+
err.span_label(*entry.get(), format!("previous definition of `{}` here", ident));
225+
err.span_label(span, "duplicate definition");
235226
err.emit();
236227
}
237228
Vacant(entry) => {
238-
entry.insert(impl_item.span);
229+
entry.insert(span);
239230
}
240231
}
241232
}

Diff for: compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs

+14-27
Original file line numberDiff line numberDiff line change
@@ -79,19 +79,19 @@ use rustc_middle::ty::{self, TyCtxt, TypeFoldable};
7979
use rustc_span::Span;
8080
use rustc_trait_selection::traits::{self, translate_substs, wf};
8181

82-
pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: DefId, span: Span) {
82+
pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
8383
if let Some(node) = parent_specialization_node(tcx, impl_def_id) {
8484
tcx.infer_ctxt().enter(|infcx| {
85-
check_always_applicable(&infcx, impl_def_id, node, span);
85+
check_always_applicable(&infcx, impl_def_id, node);
8686
});
8787
}
8888
}
8989

90-
fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: DefId) -> Option<Node> {
90+
fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Option<Node> {
9191
let trait_ref = tcx.impl_trait_ref(impl1_def_id)?;
9292
let trait_def = tcx.trait_def(trait_ref.def_id);
9393

94-
let impl2_node = trait_def.ancestors(tcx, impl1_def_id).ok()?.nth(1)?;
94+
let impl2_node = trait_def.ancestors(tcx, impl1_def_id.to_def_id()).ok()?.nth(1)?;
9595

9696
let always_applicable_trait =
9797
matches!(trait_def.specialization_kind, TraitSpecializationKind::AlwaysApplicable);
@@ -103,15 +103,8 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: DefId) -> Option<No
103103
}
104104

105105
/// Check that `impl1` is a sound specialization
106-
fn check_always_applicable(
107-
infcx: &InferCtxt<'_, '_>,
108-
impl1_def_id: DefId,
109-
impl2_node: Node,
110-
span: Span,
111-
) {
112-
if let Some((impl1_substs, impl2_substs)) =
113-
get_impl_substs(infcx, impl1_def_id, impl2_node, span)
114-
{
106+
fn check_always_applicable(infcx: &InferCtxt<'_, '_>, impl1_def_id: LocalDefId, impl2_node: Node) {
107+
if let Some((impl1_substs, impl2_substs)) = get_impl_substs(infcx, impl1_def_id, impl2_node) {
115108
let impl2_def_id = impl2_node.def_id();
116109
debug!(
117110
"check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)",
@@ -126,17 +119,10 @@ fn check_always_applicable(
126119
unconstrained_parent_impl_substs(tcx, impl2_def_id, impl2_substs)
127120
};
128121

122+
let span = tcx.def_span(impl1_def_id);
129123
check_static_lifetimes(tcx, &parent_substs, span);
130124
check_duplicate_params(tcx, impl1_substs, &parent_substs, span);
131-
132-
check_predicates(
133-
infcx,
134-
impl1_def_id.expect_local(),
135-
impl1_substs,
136-
impl2_node,
137-
impl2_substs,
138-
span,
139-
);
125+
check_predicates(infcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span);
140126
}
141127
}
142128

@@ -152,20 +138,21 @@ fn check_always_applicable(
152138
/// Would return `S1 = [C]` and `S2 = [Vec<C>, C]`.
153139
fn get_impl_substs<'tcx>(
154140
infcx: &InferCtxt<'_, 'tcx>,
155-
impl1_def_id: DefId,
141+
impl1_def_id: LocalDefId,
156142
impl2_node: Node,
157-
span: Span,
158143
) -> Option<(SubstsRef<'tcx>, SubstsRef<'tcx>)> {
159144
let tcx = infcx.tcx;
160145
let param_env = tcx.param_env(impl1_def_id);
161146

162-
let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id);
163-
let impl2_substs = translate_substs(infcx, param_env, impl1_def_id, impl1_substs, impl2_node);
147+
let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id.to_def_id());
148+
let impl2_substs =
149+
translate_substs(infcx, param_env, impl1_def_id.to_def_id(), impl1_substs, impl2_node);
164150

165151
// Conservatively use an empty `ParamEnv`.
166152
let outlives_env = OutlivesEnvironment::new(ty::ParamEnv::empty());
167-
infcx.resolve_regions_and_report_errors(impl1_def_id, &outlives_env);
153+
infcx.resolve_regions_and_report_errors(impl1_def_id.to_def_id(), &outlives_env);
168154
let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
155+
let span = tcx.def_span(impl1_def_id);
169156
tcx.sess.emit_err(SubstsOnOverriddenImpl { span });
170157
return None;
171158
};

Diff for: src/test/ui/associated-item/associated-item-duplicate-names-2.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error[E0201]: duplicate definitions with name `bar`:
44
LL | const bar: bool = true;
55
| ----------------------- previous definition of `bar` here
66
LL | fn bar() {}
7-
| ^^^^^^^^^^^ duplicate definition
7+
| ^^^^^^^^ duplicate definition
88

99
error: aborting due to previous error
1010

Diff for: src/test/ui/error-codes/E0201.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@ error[E0201]: duplicate definitions with name `bar`:
22
--> $DIR/E0201.rs:5:5
33
|
44
LL | fn bar(&self) -> bool { self.0 > 5 }
5-
| ------------------------------------ previous definition of `bar` here
5+
| --------------------- previous definition of `bar` here
66
LL | fn bar() {}
7-
| ^^^^^^^^^^^ duplicate definition
7+
| ^^^^^^^^ duplicate definition
88

99
error[E0201]: duplicate definitions with name `baz`:
1010
--> $DIR/E0201.rs:17:5
1111
|
1212
LL | fn baz(&self) -> bool { true }
13-
| ------------------------------ previous definition of `baz` here
13+
| --------------------- previous definition of `baz` here
1414
LL | fn baz(&self) -> bool { self.0 > 5 }
15-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
15+
| ^^^^^^^^^^^^^^^^^^^^^ duplicate definition
1616

1717
error[E0201]: duplicate definitions with name `Quux`:
1818
--> $DIR/E0201.rs:18:5

Diff for: src/test/ui/impl-duplicate-methods.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `orange`:
22
--> $DIR/impl-duplicate-methods.rs:5:5
33
|
44
LL | fn orange(&self) {}
5-
| ------------------- previous definition of `orange` here
5+
| ---------------- previous definition of `orange` here
66
LL | fn orange(&self) {}
7-
| ^^^^^^^^^^^^^^^^^^^ duplicate definition
7+
| ^^^^^^^^^^^^^^^^ duplicate definition
88

99
error: aborting due to previous error
1010

Diff for: src/test/ui/issues/issue-4265.stderr

+5-8
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
error[E0201]: duplicate definitions with name `bar`:
22
--> $DIR/issue-4265.rs:10:5
33
|
4-
LL | / fn bar() {
5-
LL | | Foo { baz: 0 }.bar();
6-
LL | | }
7-
| |_____- previous definition of `bar` here
8-
LL |
9-
LL | / fn bar() {
10-
LL | | }
11-
| |_____^ duplicate definition
4+
LL | fn bar() {
5+
| -------- previous definition of `bar` here
6+
...
7+
LL | fn bar() {
8+
| ^^^^^^^^ duplicate definition
129

1310
error: aborting due to previous error
1411

Diff for: src/test/ui/methods/method-macro-backtrace.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `bar`:
22
--> $DIR/method-macro-backtrace.rs:22:5
33
|
44
LL | fn bar(&self) { }
5-
| ----------------- previous definition of `bar` here
5+
| ------------- previous definition of `bar` here
66
LL | fn bar(&self) { }
7-
| ^^^^^^^^^^^^^^^^^ duplicate definition
7+
| ^^^^^^^^^^^^^ duplicate definition
88

99
error: aborting due to previous error
1010

Diff for: src/test/ui/traits/issue-8153.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `bar`:
22
--> $DIR/issue-8153.rs:11:5
33
|
44
LL | fn bar(&self) -> isize {1}
5-
| -------------------------- previous definition of `bar` here
5+
| ---------------------- previous definition of `bar` here
66
LL | fn bar(&self) -> isize {2}
7-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
7+
| ^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
88

99
error: aborting due to previous error
1010

0 commit comments

Comments
 (0)