Skip to content

Commit

Permalink
Rollup merge of #106973 - oli-obk:tait_ice_closure_in_impl_header, r=…
Browse files Browse the repository at this point in the history
…lcnr

Don't treat closures from other crates as local

fixes #104817

r? `@lcnr`

Specialization can prefer an impl for an opaque type over a blanket impls that also matches. If the blanket impl only applies if an auto-trait applies, we look at the hidden type of the opaque type to see if that implements the auto trait. The hidden type can be a closure or generator, and thus we will end up seeing these types in coherence and have to handle them properly.
  • Loading branch information
matthiaskrgr committed Jan 20, 2023
2 parents df88f7e + 42f1f54 commit 240cc81
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 24 deletions.
42 changes: 20 additions & 22 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,12 @@ fn resolve_negative_obligation<'tcx>(
infcx.resolve_regions(&outlives_env).is_empty()
}

#[instrument(level = "debug", skip(tcx), ret)]
pub fn trait_ref_is_knowable<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
) -> Result<(), Conflict> {
debug!("trait_ref_is_knowable(trait_ref={:?})", trait_ref);
if orphan_check_trait_ref(tcx, trait_ref, InCrate::Remote).is_ok() {
if orphan_check_trait_ref(trait_ref, InCrate::Remote).is_ok() {
// A downstream or cousin crate is allowed to implement some
// substitution of this trait-ref.
return Err(Conflict::Downstream);
Expand All @@ -429,11 +429,9 @@ pub fn trait_ref_is_knowable<'tcx>(
// and if we are an intermediate owner, then we don't care
// about future-compatibility, which means that we're OK if
// we are an owner.
if orphan_check_trait_ref(tcx, trait_ref, InCrate::Local).is_ok() {
debug!("trait_ref_is_knowable: orphan check passed");
if orphan_check_trait_ref(trait_ref, InCrate::Local).is_ok() {
Ok(())
} else {
debug!("trait_ref_is_knowable: nonlocal, nonfundamental, unowned");
Err(Conflict::Upstream)
}
}
Expand All @@ -445,6 +443,7 @@ pub fn trait_ref_is_local_or_fundamental<'tcx>(
trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, sym::fundamental)
}

#[derive(Debug)]
pub enum OrphanCheckErr<'tcx> {
NonLocalInputType(Vec<(Ty<'tcx>, bool /* Is this the first input type? */)>),
UncoveredTy(Ty<'tcx>, Option<Ty<'tcx>>),
Expand All @@ -456,21 +455,20 @@ pub enum OrphanCheckErr<'tcx> {
///
/// 1. All type parameters in `Self` must be "covered" by some local type constructor.
/// 2. Some local type must appear in `Self`.
#[instrument(level = "debug", skip(tcx), ret)]
pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanCheckErr<'_>> {
debug!("orphan_check({:?})", impl_def_id);

// We only except this routine to be invoked on implementations
// of a trait, not inherent implementations.
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().subst_identity();
debug!("orphan_check: trait_ref={:?}", trait_ref);
debug!(?trait_ref);

// If the *trait* is local to the crate, ok.
if trait_ref.def_id.is_local() {
debug!("trait {:?} is local to current crate", trait_ref.def_id);
return Ok(());
}

orphan_check_trait_ref(tcx, trait_ref, InCrate::Local)
orphan_check_trait_ref(trait_ref, InCrate::Local)
}

/// Checks whether a trait-ref is potentially implementable by a crate.
Expand Down Expand Up @@ -559,21 +557,19 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
///
/// Note that this function is never called for types that have both type
/// parameters and inference variables.
#[instrument(level = "trace", ret)]
fn orphan_check_trait_ref<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
in_crate: InCrate,
) -> Result<(), OrphanCheckErr<'tcx>> {
debug!("orphan_check_trait_ref(trait_ref={:?}, in_crate={:?})", trait_ref, in_crate);

if trait_ref.needs_infer() && trait_ref.needs_subst() {
bug!(
"can't orphan check a trait ref with both params and inference variables {:?}",
trait_ref
);
}

let mut checker = OrphanChecker::new(tcx, in_crate);
let mut checker = OrphanChecker::new(in_crate);
match trait_ref.visit_with(&mut checker) {
ControlFlow::Continue(()) => Err(OrphanCheckErr::NonLocalInputType(checker.non_local_tys)),
ControlFlow::Break(OrphanCheckEarlyExit::ParamTy(ty)) => {
Expand All @@ -592,7 +588,6 @@ fn orphan_check_trait_ref<'tcx>(
}

struct OrphanChecker<'tcx> {
tcx: TyCtxt<'tcx>,
in_crate: InCrate,
in_self_ty: bool,
/// Ignore orphan check failures and exclusively search for the first
Expand All @@ -602,9 +597,8 @@ struct OrphanChecker<'tcx> {
}

impl<'tcx> OrphanChecker<'tcx> {
fn new(tcx: TyCtxt<'tcx>, in_crate: InCrate) -> Self {
fn new(in_crate: InCrate) -> Self {
OrphanChecker {
tcx,
in_crate,
in_self_ty: true,
search_first_local_ty: false,
Expand Down Expand Up @@ -697,13 +691,17 @@ impl<'tcx> TypeVisitor<'tcx> for OrphanChecker<'tcx> {
}
}
ty::Error(_) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => {
self.tcx.sess.delay_span_bug(
DUMMY_SP,
format!("ty_is_local invoked on closure or generator: {:?}", ty),
);
ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty))
ty::Closure(did, ..) | ty::Generator(did, ..) => {
if self.def_id_is_local(did) {
ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty))
} else {
self.found_non_local_ty(ty)
}
}
// This should only be created when checking whether we have to check whether some
// auto trait impl applies. There will never be multiple impls, so we can just
// act as if it were a local type here.
ty::GeneratorWitness(_) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
ty::Alias(ty::Opaque, ..) => {
// This merits some explanation.
// Normally, opaque types are not involved when performing
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/coherence/coherence-with-generator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
// Test that encountering closures during coherence does not cause issues.
#![feature(type_alias_impl_trait, generators)]
#![cfg_attr(specialized, feature(specialization))]
#![allow(incomplete_features)]

// revisions: stock specialized
// [specialized]check-pass

type OpaqueGenerator = impl Sized;
fn defining_use() -> OpaqueGenerator {
|| {
Expand All @@ -13,6 +19,6 @@ struct Wrapper<T>(T);
trait Trait {}
impl Trait for Wrapper<OpaqueGenerator> {}
impl<T: Sync> Trait for Wrapper<T> {}
//~^ ERROR conflicting implementations of trait `Trait` for type `Wrapper<OpaqueGenerator>`
//[stock]~^ ERROR conflicting implementations of trait `Trait` for type `Wrapper<OpaqueGenerator>`

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0119]: conflicting implementations of trait `Trait` for type `Wrapper<OpaqueGenerator>`
--> $DIR/coherence-with-generator.rs:15:1
--> $DIR/coherence-with-generator.rs:21:1
|
LL | impl Trait for Wrapper<OpaqueGenerator> {}
| --------------------------------------- first implementation here
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/type-alias-impl-trait/issue-104817.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(type_alias_impl_trait)]
#![cfg_attr(specialized, feature(specialization))]
#![allow(incomplete_features)]

// revisions: stock specialized
// [specialized]check-pass

trait OpaqueTrait {}
impl<T> OpaqueTrait for T {}
type OpaqueType = impl OpaqueTrait;
fn mk_opaque() -> OpaqueType {
|| 0
}
trait AnotherTrait {}
impl<T: Send> AnotherTrait for T {}
impl AnotherTrait for OpaqueType {}
//[stock]~^ conflicting implementations of trait `AnotherTrait` for type `OpaqueType`

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/type-alias-impl-trait/issue-104817.stock.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0119]: conflicting implementations of trait `AnotherTrait` for type `OpaqueType`
--> $DIR/issue-104817.rs:16:1
|
LL | impl<T: Send> AnotherTrait for T {}
| -------------------------------- first implementation here
LL | impl AnotherTrait for OpaqueType {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `OpaqueType`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0119`.

0 comments on commit 240cc81

Please sign in to comment.