From 294d0c863f7c813262edbfe6fc4dbcf49a5ab916 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 17:31:18 -0400 Subject: [PATCH] Fix coherence check for opaque type Fixes #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) --- src/librustc/traits/coherence.rs | 31 +++++++++++++++++-- .../auxiliary/foreign-crate.rs | 2 ++ .../ui/type-alias-impl-trait/coherence.rs | 25 +++++++++++++++ .../ui/type-alias-impl-trait/coherence.stderr | 9 ++++++ .../issue-63677-type-alias-coherence.rs | 21 +++++++++++++ 5 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs create mode 100644 src/test/ui/type-alias-impl-trait/coherence.rs create mode 100644 src/test/ui/type-alias-impl-trait/coherence.stderr create mode 100644 src/test/ui/type-alias-impl-trait/issue-63677-type-alias-coherence.rs diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index b6f0addd77107..80a95840a73e0 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -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 } @@ -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) } } diff --git a/src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs b/src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs new file mode 100644 index 0000000000000..52802dd8fbb47 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/auxiliary/foreign-crate.rs @@ -0,0 +1,2 @@ +pub trait ForeignTrait {} +pub struct ForeignType(pub T); diff --git a/src/test/ui/type-alias-impl-trait/coherence.rs b/src/test/ui/type-alias-impl-trait/coherence.rs new file mode 100644 index 0000000000000..9617579df6ff0 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/coherence.rs @@ -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 LocalTrait for foreign_crate::ForeignType {} + +type AliasOfForeignType = impl LocalTrait; +fn use_alias(val: T) -> AliasOfForeignType { + foreign_crate::ForeignType(val) +} + +impl foreign_crate::ForeignTrait for AliasOfForeignType {} +//~^ ERROR the type parameter `T` is not constrained by the impl trait, self type, or predicates + +fn main() {} diff --git a/src/test/ui/type-alias-impl-trait/coherence.stderr b/src/test/ui/type-alias-impl-trait/coherence.stderr new file mode 100644 index 0000000000000..44281aa684913 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/coherence.stderr @@ -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 foreign_crate::ForeignTrait for AliasOfForeignType {} + | ^ unconstrained type parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0207`. diff --git a/src/test/ui/type-alias-impl-trait/issue-63677-type-alias-coherence.rs b/src/test/ui/type-alias-impl-trait/issue-63677-type-alias-coherence.rs new file mode 100644 index 0000000000000..28f4a85c9f290 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/issue-63677-type-alias-coherence.rs @@ -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); +pub struct S2(T); + +pub type T1 = impl Trait; +pub type T2 = S1; +pub type T3 = S2; + +impl Trait for S1 {} +impl S2 {} +impl T3 {} + +pub fn use_t1() -> T1 { S1(()) } + +fn main() {}