Skip to content

Commit

Permalink
Auto merge of #98396 - cjgillot:iwfchir, r=petrochenkov
Browse files Browse the repository at this point in the history 
Do not access HIR to check impl wf.

r? `@ghost`
  • Loading branch information
@bors
bors committed Jun 28, 2022
2 parents 64eb9ab + 8242aa8 commit baf382e
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 74 deletions.
47 changes: 19 additions & 28 deletions compiler/rustc_typeck/src/impl_wf_check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use min_specialization::check_min_specialization;

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::ty::query::Providers;
Expand Down Expand Up @@ -59,13 +58,10 @@ fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
let module = tcx.hir_module_items(module_def_id);
for id in module.items() {
if matches!(tcx.def_kind(id.def_id), DefKind::Impl) {
let item = tcx.hir().item(id);
if let hir::ItemKind::Impl(ref impl_) = item.kind {
enforce_impl_params_are_constrained(tcx, item.def_id, impl_.items);
enforce_impl_items_are_distinct(tcx, impl_.items);
if min_specialization {
check_min_specialization(tcx, item.def_id.to_def_id(), item.span);
}
enforce_impl_params_are_constrained(tcx, id.def_id);
enforce_impl_items_are_distinct(tcx, id.def_id);
if min_specialization {
check_min_specialization(tcx, id.def_id);
}
}
}
Expand All @@ -75,11 +71,7 @@ pub fn provide(providers: &mut Providers) {
*providers = Providers { check_mod_impl_wf, ..*providers };
}

fn enforce_impl_params_are_constrained(
tcx: TyCtxt<'_>,
impl_def_id: LocalDefId,
impl_item_refs: &[hir::ImplItemRef],
) {
fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
// Every lifetime used in an associated type must be constrained.
let impl_self_ty = tcx.type_of(impl_def_id);
if impl_self_ty.references_error() {
Expand Down Expand Up @@ -107,9 +99,9 @@ fn enforce_impl_params_are_constrained(
);

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

/// Enforce that we do not have two items in an impl with the same name.
fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef]) {
fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
let mut seen_type_items = FxHashMap::default();
let mut seen_value_items = FxHashMap::default();
for impl_item_ref in impl_item_refs {
let impl_item = tcx.hir().impl_item(impl_item_ref.id);
for &impl_item_ref in tcx.associated_item_def_ids(impl_def_id) {
let impl_item = tcx.associated_item(impl_item_ref);
let seen_items = match impl_item.kind {
hir::ImplItemKind::TyAlias(_) => &mut seen_type_items,
ty::AssocKind::Type => &mut seen_type_items,
_ => &mut seen_value_items,
};
match seen_items.entry(impl_item.ident.normalize_to_macros_2_0()) {
let span = tcx.def_span(impl_item_ref);
let ident = impl_item.ident(tcx);
match seen_items.entry(ident.normalize_to_macros_2_0()) {
Occupied(entry) => {
let mut err = struct_span_err!(
tcx.sess,
impl_item.span,
span,
E0201,
"duplicate definitions with name `{}`:",
impl_item.ident
);
err.span_label(
*entry.get(),
format!("previous definition of `{}` here", impl_item.ident),
ident
);
err.span_label(impl_item.span, "duplicate definition");
err.span_label(*entry.get(), format!("previous definition of `{}` here", ident));
err.span_label(span, "duplicate definition");
err.emit();
}
Vacant(entry) => {
entry.insert(impl_item.span);
entry.insert(span);
}
}
}
Expand Down
41 changes: 14 additions & 27 deletions compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ use rustc_middle::ty::{self, TyCtxt, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits::{self, translate_substs, wf};

pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: DefId, span: Span) {
pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
if let Some(node) = parent_specialization_node(tcx, impl_def_id) {
tcx.infer_ctxt().enter(|infcx| {
check_always_applicable(&infcx, impl_def_id, node, span);
check_always_applicable(&infcx, impl_def_id, node);
});
}
}

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

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

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

/// Check that `impl1` is a sound specialization
fn check_always_applicable(
infcx: &InferCtxt<'_, '_>,
impl1_def_id: DefId,
impl2_node: Node,
span: Span,
) {
if let Some((impl1_substs, impl2_substs)) =
get_impl_substs(infcx, impl1_def_id, impl2_node, span)
{
fn check_always_applicable(infcx: &InferCtxt<'_, '_>, impl1_def_id: LocalDefId, impl2_node: Node) {
if let Some((impl1_substs, impl2_substs)) = get_impl_substs(infcx, impl1_def_id, impl2_node) {
let impl2_def_id = impl2_node.def_id();
debug!(
"check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)",
Expand All @@ -126,17 +119,10 @@ fn check_always_applicable(
unconstrained_parent_impl_substs(tcx, impl2_def_id, impl2_substs)
};

let span = tcx.def_span(impl1_def_id);
check_static_lifetimes(tcx, &parent_substs, span);
check_duplicate_params(tcx, impl1_substs, &parent_substs, span);

check_predicates(
infcx,
impl1_def_id.expect_local(),
impl1_substs,
impl2_node,
impl2_substs,
span,
);
check_predicates(infcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span);
}
}

Expand All @@ -152,20 +138,21 @@ fn check_always_applicable(
/// Would return `S1 = [C]` and `S2 = [Vec<C>, C]`.
fn get_impl_substs<'tcx>(
infcx: &InferCtxt<'_, 'tcx>,
impl1_def_id: DefId,
impl1_def_id: LocalDefId,
impl2_node: Node,
span: Span,
) -> Option<(SubstsRef<'tcx>, SubstsRef<'tcx>)> {
let tcx = infcx.tcx;
let param_env = tcx.param_env(impl1_def_id);

let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id);
let impl2_substs = translate_substs(infcx, param_env, impl1_def_id, impl1_substs, impl2_node);
let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id.to_def_id());
let impl2_substs =
translate_substs(infcx, param_env, impl1_def_id.to_def_id(), impl1_substs, impl2_node);

// Conservatively use an empty `ParamEnv`.
let outlives_env = OutlivesEnvironment::new(ty::ParamEnv::empty());
infcx.resolve_regions_and_report_errors(impl1_def_id, &outlives_env);
infcx.resolve_regions_and_report_errors(impl1_def_id.to_def_id(), &outlives_env);
let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
let span = tcx.def_span(impl1_def_id);
tcx.sess.emit_err(SubstsOnOverriddenImpl { span });
return None;
};
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/associated-item/associated-item-duplicate-names-2.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0201]: duplicate definitions with name `bar`:
LL | const bar: bool = true;
| ----------------------- previous definition of `bar` here
LL | fn bar() {}
| ^^^^^^^^^^^ duplicate definition
| ^^^^^^^^ duplicate definition

error: aborting due to previous error

Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/error-codes/E0201.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ error[E0201]: duplicate definitions with name `bar`:
--> $DIR/E0201.rs:5:5
|
LL | fn bar(&self) -> bool { self.0 > 5 }
| ------------------------------------ previous definition of `bar` here
| --------------------- previous definition of `bar` here
LL | fn bar() {}
| ^^^^^^^^^^^ duplicate definition
| ^^^^^^^^ duplicate definition

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

error[E0201]: duplicate definitions with name `Quux`:
--> $DIR/E0201.rs:18:5
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/impl-duplicate-methods.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `orange`:
--> $DIR/impl-duplicate-methods.rs:5:5
|
LL | fn orange(&self) {}
| ------------------- previous definition of `orange` here
| ---------------- previous definition of `orange` here
LL | fn orange(&self) {}
| ^^^^^^^^^^^^^^^^^^^ duplicate definition
| ^^^^^^^^^^^^^^^^ duplicate definition

error: aborting due to previous error

Expand Down
13 changes: 5 additions & 8 deletions src/test/ui/issues/issue-4265.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
error[E0201]: duplicate definitions with name `bar`:
--> $DIR/issue-4265.rs:10:5
|
LL | / fn bar() {
LL | | Foo { baz: 0 }.bar();
LL | | }
| |_____- previous definition of `bar` here
LL |
LL | / fn bar() {
LL | | }
| |_____^ duplicate definition
LL | fn bar() {
| -------- previous definition of `bar` here
...
LL | fn bar() {
| ^^^^^^^^ duplicate definition

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/methods/method-macro-backtrace.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `bar`:
--> $DIR/method-macro-backtrace.rs:22:5
|
LL | fn bar(&self) { }
| ----------------- previous definition of `bar` here
| ------------- previous definition of `bar` here
LL | fn bar(&self) { }
| ^^^^^^^^^^^^^^^^^ duplicate definition
| ^^^^^^^^^^^^^ duplicate definition

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/traits/issue-8153.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ error[E0201]: duplicate definitions with name `bar`:
--> $DIR/issue-8153.rs:11:5
|
LL | fn bar(&self) -> isize {1}
| -------------------------- previous definition of `bar` here
| ---------------------- previous definition of `bar` here
LL | fn bar(&self) -> isize {2}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
| ^^^^^^^^^^^^^^^^^^^^^^ duplicate definition

error: aborting due to previous error

Expand Down

0 comments on commit baf382e

Please sign in to comment.