Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform coherence checking per impl. #98221

Merged
merged 3 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,9 +906,10 @@ rustc_queries! {

/// Checks whether all impls in the crate pass the overlap check, returning
/// which impls fail it. If all impls are correct, the returned slice is empty.
query orphan_check_crate(_: ()) -> &'tcx [LocalDefId] {
desc {
"checking whether the immpl in the this crate follow the orphan rules",
query orphan_check_impl(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
desc { |tcx|
"checking whether impl `{}` follows the orphan rules",
tcx.def_path_str(key.to_def_id()),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ fn report_conflicting_impls(
match used_to_be_allowed {
None => {
let reported = if overlap.with_impl.is_local()
|| !tcx.orphan_check_crate(()).contains(&impl_def_id)
|| tcx.orphan_check_impl(impl_def_id).is_ok()
{
let err = struct_span_err!(tcx.sess, impl_span, E0119, "");
Some(decorate(
Expand Down
19 changes: 5 additions & 14 deletions compiler/rustc_typeck/src/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub fn provide(providers: &mut Providers) {
use self::builtin::coerce_unsized_info;
use self::inherent_impls::{crate_incoherent_impls, crate_inherent_impls, inherent_impls};
use self::inherent_impls_overlap::crate_inherent_impls_overlap_check;
use self::orphan::orphan_check_crate;
use self::orphan::orphan_check_impl;

*providers = Providers {
coherent_trait,
Expand All @@ -155,7 +155,7 @@ pub fn provide(providers: &mut Providers) {
inherent_impls,
crate_inherent_impls_overlap_check,
coerce_unsized_info,
orphan_check_crate,
orphan_check_impl,
..*providers
};
}
Expand All @@ -171,21 +171,12 @@ fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) {

check_impl(tcx, impl_def_id, trait_ref);
check_object_overlap(tcx, impl_def_id, trait_ref);
}
builtin::check_trait(tcx, def_id);
}

pub fn check_coherence(tcx: TyCtxt<'_>) {
tcx.sess.time("unsafety_checking", || unsafety::check(tcx));
tcx.ensure().orphan_check_crate(());

for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
tcx.ensure().coherent_trait(trait_def_id);
tcx.sess.time("unsafety_checking", || unsafety::check_item(tcx, impl_def_id));
tcx.sess.time("orphan_checking", || tcx.ensure().orphan_check_impl(impl_def_id));
}

// these queries are executed for side-effects (error reporting):
tcx.ensure().crate_inherent_impls(());
tcx.ensure().crate_inherent_impls_overlap_check(());
builtin::check_trait(tcx, def_id);
}

/// Checks whether an impl overlaps with the automatic `impl Trait for dyn Trait`.
Expand Down
176 changes: 86 additions & 90 deletions compiler/rustc_typeck/src/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,29 @@ use rustc_span::Span;
use rustc_trait_selection::traits;
use std::ops::ControlFlow;

pub(super) fn orphan_check_crate(tcx: TyCtxt<'_>, (): ()) -> &[LocalDefId] {
let mut errors = Vec::new();
for (&trait_def_id, impls_of_trait) in tcx.all_local_trait_impls(()) {
for &impl_of_trait in impls_of_trait {
match orphan_check_impl(tcx, impl_of_trait) {
Ok(()) => {}
Err(_) => errors.push(impl_of_trait),
}
}
#[instrument(skip(tcx), level = "debug")]
pub(crate) fn orphan_check_impl(
tcx: TyCtxt<'_>,
impl_def_id: LocalDefId,
) -> Result<(), ErrorGuaranteed> {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
if let Some(err) = trait_ref.error_reported() {
return Err(err);
}

if tcx.trait_is_auto(trait_def_id) {
lint_auto_trait_impls(tcx, trait_def_id, impls_of_trait);
}
let ret = do_orphan_check_impl(tcx, trait_ref, impl_def_id);
if tcx.trait_is_auto(trait_ref.def_id) {
lint_auto_trait_impl(tcx, trait_ref, impl_def_id);
}
tcx.arena.alloc_slice(&errors)

ret
}

#[instrument(skip(tcx), level = "debug")]
fn orphan_check_impl(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGuaranteed> {
let trait_ref = tcx.impl_trait_ref(def_id).unwrap();
fn do_orphan_check_impl<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
def_id: LocalDefId,
) -> Result<(), ErrorGuaranteed> {
let trait_def_id = trait_ref.def_id;

let item = tcx.hir().item(hir::ItemId { def_id });
Expand Down Expand Up @@ -329,89 +332,82 @@ fn emit_orphan_check_error<'tcx>(

/// Lint impls of auto traits if they are likely to have
/// unsound or surprising effects on auto impls.
fn lint_auto_trait_impls(tcx: TyCtxt<'_>, trait_def_id: DefId, impls: &[LocalDefId]) {
let mut non_covering_impls = Vec::new();
for &impl_def_id in impls {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
if trait_ref.references_error() {
return;
}
fn lint_auto_trait_impl<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
impl_def_id: LocalDefId,
) {
if tcx.impl_polarity(impl_def_id) != ImplPolarity::Positive {
return;
}

if tcx.impl_polarity(impl_def_id) != ImplPolarity::Positive {
assert_eq!(trait_ref.substs.len(), 1);
let self_ty = trait_ref.self_ty();
let (self_type_did, substs) = match self_ty.kind() {
ty::Adt(def, substs) => (def.did(), substs),
_ => {
// FIXME: should also lint for stuff like `&i32` but
// considering that auto traits are unstable, that
// isn't too important for now as this only affects
// crates using `nightly`, and std.
return;
}
};

assert_eq!(trait_ref.substs.len(), 1);
let self_ty = trait_ref.self_ty();
let (self_type_did, substs) = match self_ty.kind() {
ty::Adt(def, substs) => (def.did(), substs),
_ => {
// FIXME: should also lint for stuff like `&i32` but
// considering that auto traits are unstable, that
// isn't too important for now as this only affects
// crates using `nightly`, and std.
continue;
}
};
// Impls which completely cover a given root type are fine as they
// disable auto impls entirely. So only lint if the substs
// are not a permutation of the identity substs.
let Err(arg) = tcx.uses_unique_generic_params(substs, IgnoreRegions::Yes) else {
// ok
return;
};

// Impls which completely cover a given root type are fine as they
// disable auto impls entirely. So only lint if the substs
// are not a permutation of the identity substs.
match tcx.uses_unique_generic_params(substs, IgnoreRegions::Yes) {
Ok(()) => {} // ok
Err(arg) => {
// Ideally:
//
// - compute the requirements for the auto impl candidate
// - check whether these are implied by the non covering impls
// - if not, emit the lint
//
// What we do here is a bit simpler:
//
// - badly check if an auto impl candidate definitely does not apply
// for the given simplified type
// - if so, do not lint
if fast_reject_auto_impl(tcx, trait_def_id, self_ty) {
// ok
} else {
non_covering_impls.push((impl_def_id, self_type_did, arg));
}
}
}
// Ideally:
//
// - compute the requirements for the auto impl candidate
// - check whether these are implied by the non covering impls
// - if not, emit the lint
//
// What we do here is a bit simpler:
//
// - badly check if an auto impl candidate definitely does not apply
// for the given simplified type
// - if so, do not lint
if fast_reject_auto_impl(tcx, trait_ref.def_id, self_ty) {
// ok
return;
}

for &(impl_def_id, self_type_did, arg) in &non_covering_impls {
tcx.struct_span_lint_hir(
lint::builtin::SUSPICIOUS_AUTO_TRAIT_IMPLS,
tcx.hir().local_def_id_to_hir_id(impl_def_id),
tcx.def_span(impl_def_id),
|err| {
let item_span = tcx.def_span(self_type_did);
let self_descr = tcx.def_kind(self_type_did).descr(self_type_did);
let mut err = err.build(&format!(
"cross-crate traits with a default impl, like `{}`, \
tcx.struct_span_lint_hir(
lint::builtin::SUSPICIOUS_AUTO_TRAIT_IMPLS,
tcx.hir().local_def_id_to_hir_id(impl_def_id),
tcx.def_span(impl_def_id),
|err| {
let item_span = tcx.def_span(self_type_did);
let self_descr = tcx.def_kind(self_type_did).descr(self_type_did);
let mut err = err.build(&format!(
"cross-crate traits with a default impl, like `{}`, \
should not be specialized",
tcx.def_path_str(trait_def_id),
));
match arg {
ty::util::NotUniqueParam::DuplicateParam(arg) => {
err.note(&format!("`{}` is mentioned multiple times", arg));
}
ty::util::NotUniqueParam::NotParam(arg) => {
err.note(&format!("`{}` is not a generic parameter", arg));
}
tcx.def_path_str(trait_ref.def_id),
));
match arg {
ty::util::NotUniqueParam::DuplicateParam(arg) => {
err.note(&format!("`{}` is mentioned multiple times", arg));
}
err.span_note(
item_span,
&format!(
"try using the same sequence of generic parameters as the {} definition",
self_descr,
),
);
err.emit();
},
);
}
ty::util::NotUniqueParam::NotParam(arg) => {
err.note(&format!("`{}` is not a generic parameter", arg));
}
}
err.span_note(
item_span,
&format!(
"try using the same sequence of generic parameters as the {} definition",
self_descr,
),
);
err.emit();
},
);
}

fn fast_reject_auto_impl<'tcx>(tcx: TyCtxt<'tcx>, trait_def_id: DefId, self_ty: Ty<'tcx>) -> bool {
Expand Down
35 changes: 8 additions & 27 deletions compiler/rustc_typeck/src/coherence/unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,18 @@ use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::Unsafety;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::LocalDefId;

pub fn check(tcx: TyCtxt<'_>) {
for id in tcx.hir().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 {
check_unsafety_coherence(
tcx,
item,
Some(&impl_.generics),
impl_.unsafety,
impl_.polarity,
);
}
}
}
}
pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
debug_assert!(matches!(tcx.def_kind(def_id), DefKind::Impl));
let item = tcx.hir().expect_item(def_id);
let hir::ItemKind::Impl(ref impl_) = item.kind else { bug!() };

fn check_unsafety_coherence<'tcx>(
tcx: TyCtxt<'tcx>,
item: &hir::Item<'_>,
impl_generics: Option<&hir::Generics<'_>>,
unsafety: hir::Unsafety,
polarity: hir::ImplPolarity,
) {
if let Some(trait_ref) = tcx.impl_trait_ref(item.def_id) {
let trait_def = tcx.trait_def(trait_ref.def_id);
let unsafe_attr = impl_generics.and_then(|generics| {
generics.params.iter().find(|p| p.pure_wrt_drop).map(|_| "may_dangle")
});
match (trait_def.unsafety, unsafe_attr, unsafety, polarity) {
let unsafe_attr =
impl_.generics.params.iter().find(|p| p.pure_wrt_drop).map(|_| "may_dangle");
match (trait_def.unsafety, unsafe_attr, impl_.unsafety, impl_.polarity) {
(Unsafety::Normal, None, Unsafety::Unsafe, hir::ImplPolarity::Positive) => {
struct_span_err!(
tcx.sess,
Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,15 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
})?;

tcx.sess.track_errors(|| {
tcx.sess.time("coherence_checking", || coherence::check_coherence(tcx));
tcx.sess.time("coherence_checking", || {
for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
tcx.ensure().coherent_trait(trait_def_id);
}

// these queries are executed for side-effects (error reporting):
tcx.ensure().crate_inherent_impls(());
tcx.ensure().crate_inherent_impls_overlap_check(());
});
})?;

if tcx.features().rustc_attrs {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1`
--> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
|
LL | impl !Marker1 for dyn Object + Marker2 { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1`

error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2`
--> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:17:1
|
LL | impl !Marker2 for dyn Object + Marker2 { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2`

error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
--> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:23:1
|
Expand All @@ -21,18 +33,6 @@ error[E0321]: cross-crate traits with a default impl, like `Send`, can only be i
LL | impl !Send for dyn Object + Marker2 {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type

error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1`
--> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
|
LL | impl !Marker1 for dyn Object + Marker2 { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1`

error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2`
--> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:17:1
|
LL | impl !Marker2 for dyn Object + Marker2 { }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2`

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0117, E0321, E0371.
Expand Down
Loading