Skip to content

Commit 294d0c8

Browse files
committed
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)
1 parent dece573 commit 294d0c8

File tree

5 files changed

+85
-3
lines changed

5 files changed

+85
-3
lines changed

src/librustc/traits/coherence.rs

+28-3
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,33 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
491491
ty::Never |
492492
ty::Tuple(..) |
493493
ty::Param(..) |
494-
ty::Projection(..) => {
494+
ty::Projection(..) |
495+
// This merits some explanation.
496+
// Normally, opaque types are not involed when performing
497+
// coherence checking, since it is illegal to directly
498+
// implement a trait on an opaque type. However, we might
499+
// end up looking at an opaque type during coherence checking
500+
// if an opaque type gets used within another type (e.g. as
501+
// a type parameter). This requires us to decide whether or
502+
// not an opaque type should be considered 'local' or not.
503+
//
504+
// We choose to treat all opaque types as non-local, even
505+
// those that appear within the same crate. This seems
506+
// somewhat suprising at first, but makes sense when
507+
// you consider that opaque types are supposed to hide
508+
// the underlying type *within the same crate*. When an
509+
// opaque type is used from outside the module
510+
// where it is declared, it should be impossible to observe
511+
// anyything about it other than the traits that it implements.
512+
//
513+
// The alternative would be to look at the underlying type
514+
// to determine whether or not the opaque type itself should
515+
// be considered local. However, this could make it a breaking change
516+
// to switch the underlying ('defining') type from a local type
517+
// to a remote type. This would violate the rule that opaque
518+
// types should be completely opaque apart from the traits
519+
// that they implement, so we don't use this behavior.
520+
ty::Opaque(..) => {
495521
false
496522
}
497523

@@ -518,8 +544,7 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
518544
ty::UnnormalizedProjection(..) |
519545
ty::Closure(..) |
520546
ty::Generator(..) |
521-
ty::GeneratorWitness(..) |
522-
ty::Opaque(..) => {
547+
ty::GeneratorWitness(..) => {
523548
bug!("ty_is_local invoked on unexpected type: {:?}", ty)
524549
}
525550
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
pub trait ForeignTrait {}
2+
pub struct ForeignType<T>(pub T);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// aux-build:foreign-crate.rs
2+
// This test ensures that an opaque type cannot be used
3+
// to bypass the normal orphan impl rules.
4+
// Specifically, it should not be possible to implement
5+
// a trait for a local opaque type which resolves to a foreign type.
6+
//
7+
// This should also be prevented by the fact that writing impls for opaque
8+
// types is not allowed at all, but this test makes sure to test
9+
// the orphan rule specifically
10+
#![feature(type_alias_impl_trait)]
11+
12+
extern crate foreign_crate;
13+
14+
trait LocalTrait {}
15+
impl<T> LocalTrait for foreign_crate::ForeignType<T> {}
16+
17+
type AliasOfForeignType<T> = impl LocalTrait;
18+
fn use_alias<T>(val: T) -> AliasOfForeignType<T> {
19+
foreign_crate::ForeignType(val)
20+
}
21+
22+
impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
23+
//~^ ERROR the type parameter `T` is not constrained by the impl trait, self type, or predicates
24+
25+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
error[E0207]: the type parameter `T` is not constrained by the impl trait, self type, or predicates
2+
--> $DIR/coherence.rs:22:6
3+
|
4+
LL | impl<T> foreign_crate::ForeignTrait for AliasOfForeignType<T> {}
5+
| ^ unconstrained type parameter
6+
7+
error: aborting due to previous error
8+
9+
For more information about this error, try `rustc --explain E0207`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// check-pass
2+
// Regression test for issue #63677 - ensure that
3+
// coherence checking can properly handle 'impl trait'
4+
// in type aliases
5+
#![feature(type_alias_impl_trait)]
6+
7+
pub trait Trait {}
8+
pub struct S1<T>(T);
9+
pub struct S2<T>(T);
10+
11+
pub type T1 = impl Trait;
12+
pub type T2 = S1<T1>;
13+
pub type T3 = S2<T2>;
14+
15+
impl<T> Trait for S1<T> {}
16+
impl<T: Trait> S2<T> {}
17+
impl T3 {}
18+
19+
pub fn use_t1() -> T1 { S1(()) }
20+
21+
fn main() {}

0 commit comments

Comments
 (0)