Skip to content

Commit

Permalink
Fix coherence check for opaque type
Browse files Browse the repository at this point in the history
Fixes rust-lang#63677

This commit treats all opaque types as 'remote' with respect to
coherence checking, instead of causing an ICE. This ensures that opaque
types cannot ever 'leak' information about the underlying type (e.g.
whether or not it is a local or remote type)
  • Loading branch information
Aaron1011 committed Sep 18, 2019
1 parent dece573 commit 294d0c8
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 3 deletions.
31 changes: 28 additions & 3 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,33 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
ty::Never |
ty::Tuple(..) |
ty::Param(..) |
ty::Projection(..) => {
ty::Projection(..) |
// This merits some explanation.
// Normally, opaque types are not involed 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
// a type parameter). 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 suprising 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
// anyything 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.
ty::Opaque(..) => {
false
}

Expand All @@ -518,8 +544,7 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
ty::UnnormalizedProjection(..) |
ty::Closure(..) |
ty::Generator(..) |
ty::GeneratorWitness(..) |
ty::Opaque(..) => {
ty::GeneratorWitness(..) => {
bug!("ty_is_local invoked on unexpected type: {:?}", ty)
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub trait ForeignTrait {}
pub struct ForeignType<T>(pub T);
25 changes: 25 additions & 0 deletions src/test/ui/type-alias-impl-trait/coherence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// aux-build:foreign-crate.rs
// This test ensures that an opaque type cannot be used
// to bypass the normal orphan impl rules.
// Specifically, it should not be possible to implement
// a trait for a local opaque type which resolves to a foreign type.
//
// This should also be prevented by the fact that writing impls for opaque
// types is not allowed at all, but this test makes sure to test
// the orphan rule specifically
#![feature(type_alias_impl_trait)]

extern crate foreign_crate;

trait LocalTrait {}
impl<T> LocalTrait for foreign_crate::ForeignType<T> {}

type AliasOfForeignType<T> = impl LocalTrait;
fn use_alias<T>(val: T) -> AliasOfForeignType<T> {
foreign_crate::ForeignType(val)
}

impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
//~^ ERROR the type parameter `T` is not constrained by the impl trait, self type, or predicates

fn main() {}
9 changes: 9 additions & 0 deletions src/test/ui/type-alias-impl-trait/coherence.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0207]: the type parameter `T` is not constrained by the impl trait, self type, or predicates
--> $DIR/coherence.rs:22:6
|
LL | impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
| ^ unconstrained type parameter

error: aborting due to previous error

For more information about this error, try `rustc --explain E0207`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// check-pass
// Regression test for issue #63677 - ensure that
// coherence checking can properly handle 'impl trait'
// in type aliases
#![feature(type_alias_impl_trait)]

pub trait Trait {}
pub struct S1<T>(T);
pub struct S2<T>(T);

pub type T1 = impl Trait;
pub type T2 = S1<T1>;
pub type T3 = S2<T2>;

impl<T> Trait for S1<T> {}
impl<T: Trait> S2<T> {}
impl T3 {}

pub fn use_t1() -> T1 { S1(()) }

fn main() {}

0 comments on commit 294d0c8

Please sign in to comment.