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

Restore Opaque behavior to coherence check #99666

Merged
merged 1 commit into from
Jul 26, 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
31 changes: 30 additions & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,13 +703,42 @@ impl<'tcx> TypeVisitor<'tcx> for OrphanChecker<'tcx> {
}
}
ty::Error(_) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
ty::Opaque(..) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => {
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::Opaque(..) => {
// This merits some explanation.
// Normally, opaque types are not involved when performing
// coherence checking, since it is illegal to directly
// implement a trait on an opaque type. However, we might
// end up looking at an opaque type during coherence checking
// if an opaque type gets used within another type (e.g. as
// the type of a field) when checking for auto trait or `Sized`
// impls. This requires us to decide whether or not an opaque
// type should be considered 'local' or not.
//
// We choose to treat all opaque types as non-local, even
// those that appear within the same crate. This seems
// somewhat surprising at first, but makes sense when
// you consider that opaque types are supposed to hide
// the underlying type *within the same crate*. When an
// opaque type is used from outside the module
// where it is declared, it should be impossible to observe
// anything about it other than the traits that it implements.
//
// The alternative would be to look at the underlying type
// to determine whether or not the opaque type itself should
// be considered local. However, this could make it a breaking change
// to switch the underlying ('defining') type from a local type
// to a remote type. This would violate the rule that opaque
// types should be completely opaque apart from the traits
// that they implement, so we don't use this behavior.
self.found_non_local_ty(ty)
}
};
// A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so
// the first type we visit is always the self type.
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/coherence/issue-99663-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass

#![feature(type_alias_impl_trait)]

struct Outer<T: ?Sized> {
i: InnerSend<T>,
}

type InnerSend<T: ?Sized> = impl Send;

fn constrain<T: ?Sized>() -> InnerSend<T> {
()
}

trait SendMustNotImplDrop {}

#[allow(drop_bounds)]
impl<T: ?Sized + Send + Drop> SendMustNotImplDrop for T {}

impl<T: ?Sized> SendMustNotImplDrop for Outer<T> {}

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/coherence/issue-99663.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass

#![feature(type_alias_impl_trait)]

struct Send<T> {
i: InnerSend<T>,
}

type InnerSend<T> = impl Sized;

fn constrain<T>() -> InnerSend<T> {
()
}

trait SendMustNotImplDrop {}

#[allow(drop_bounds)]
impl<T: Drop> SendMustNotImplDrop for T {}

impl<T> SendMustNotImplDrop for Send<T> {}

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/impl-trait/negative-reasoning.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | impl<T: std::fmt::Debug> AnotherTrait for T {}
LL | impl AnotherTrait for D<OpaqueType> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<OpaqueType>`
|
= note: downstream crates may implement trait `std::fmt::Debug` for type `OpaqueType`
= note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `OpaqueType` in future versions

error: cannot implement trait on type alias impl trait
--> $DIR/negative-reasoning.rs:19:25
Expand Down