Skip to content

Commit f801823

Browse files
fmeasefmease
committed
Orphanck: Consider opaque types to never cover type parameters
1 parent 0a59f11 commit f801823

7 files changed

+124
-32
lines changed

‎compiler/rustc_trait_selection/src/traits/coherence.rs

+37-31
Original file line numberDiff line numberDiff line change
@@ -924,9 +924,9 @@ where
924924
}
925925
}
926926

927-
ty::Alias(kind @ (ty::Projection | ty::Inherent | ty::Weak), ..) => {
927+
ty::Alias(kind @ (ty::Projection | ty::Inherent | ty::Weak), _) => {
928928
if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
929-
bug!("unexpected ty param in alias ty");
929+
bug!("orphanck: unexpected ty param in alias ty");
930930
}
931931

932932
if ty.has_type_flags(
@@ -952,6 +952,41 @@ where
952952
}
953953
}
954954

955+
ty::Alias(ty::Opaque, _) => {
956+
// This merits some explanation.
957+
//
958+
// We choose to treat all opaque types as non-local, even
959+
// those that appear within the same crate. This seems
960+
// somewhat surprising at first, but makes sense when
961+
// you consider that opaque types are supposed to hide
962+
// the underlying type *within the same crate*. When an
963+
// opaque type is used from outside the module
964+
// where it is declared, it should be impossible to observe
965+
// anything about it other than the traits that it implements.
966+
//
967+
// The alternative would be to look at the underlying type
968+
// to determine whether or not the opaque type itself should
969+
// be considered local. However, this could make it a breaking change
970+
// to switch the underlying ('defining') type from a local type
971+
// to a remote type. This would violate the rule that opaque
972+
// types should be completely opaque apart from the traits
973+
// that they implement, so we don't use this behavior.
974+
975+
if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
976+
bug!("orphanck: unexpected ty param in opaque ty");
977+
}
978+
979+
if ty.has_type_flags(
980+
ty::TypeFlags::HAS_TY_PLACEHOLDER
981+
| ty::TypeFlags::HAS_TY_BOUND
982+
| ty::TypeFlags::HAS_TY_INFER,
983+
) {
984+
self.found_uncovered_ty_param(ty)
985+
} else {
986+
self.found_non_local_ty(ty)
987+
}
988+
}
989+
955990
// For fundamental types, we just look inside of them.
956991
ty::Ref(_, ty, _) => ty.visit_with(self),
957992
ty::Adt(def, args) => {
@@ -990,35 +1025,6 @@ where
9901025
// auto trait impl applies. There will never be multiple impls, so we can just
9911026
// act as if it were a local type here.
9921027
ty::CoroutineWitness(..) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
993-
ty::Alias(ty::Opaque, ..) => {
994-
// This merits some explanation.
995-
// Normally, opaque types are not involved when performing
996-
// coherence checking, since it is illegal to directly
997-
// implement a trait on an opaque type. However, we might
998-
// end up looking at an opaque type during coherence checking
999-
// if an opaque type gets used within another type (e.g. as
1000-
// the type of a field) when checking for auto trait or `Sized`
1001-
// impls. This requires us to decide whether or not an opaque
1002-
// type should be considered 'local' or not.
1003-
//
1004-
// We choose to treat all opaque types as non-local, even
1005-
// those that appear within the same crate. This seems
1006-
// somewhat surprising at first, but makes sense when
1007-
// you consider that opaque types are supposed to hide
1008-
// the underlying type *within the same crate*. When an
1009-
// opaque type is used from outside the module
1010-
// where it is declared, it should be impossible to observe
1011-
// anything about it other than the traits that it implements.
1012-
//
1013-
// The alternative would be to look at the underlying type
1014-
// to determine whether or not the opaque type itself should
1015-
// be considered local. However, this could make it a breaking change
1016-
// to switch the underlying ('defining') type from a local type
1017-
// to a remote type. This would violate the rule that opaque
1018-
// types should be completely opaque apart from the traits
1019-
// that they implement, so we don't use this behavior.
1020-
self.found_non_local_ty(ty)
1021-
}
10221028
};
10231029
// A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so
10241030
// the first type we visit is always the self type.

‎tests/ui/coherence/orphan-check-opaque-types-not-covering.classic.stderr

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
2+
--> $DIR/orphan-check-opaque-types-not-covering.rs:17:6
3+
|
4+
LL | impl<T> foreign::Trait0<Local, T, ()> for Identity<T> {}
5+
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
6+
|
7+
= note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
8+
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
9+
10+
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
11+
--> $DIR/orphan-check-opaque-types-not-covering.rs:26:6
12+
|
13+
LL | impl<T> foreign::Trait1<Local, T> for Opaque<T> {}
14+
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
15+
|
16+
= note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
17+
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
18+
19+
error: aborting due to 2 previous errors
20+
21+
For more information about this error, try `rustc --explain E0210`.

‎tests/ui/coherence/orphan-check-opaque-types-not-covering.next.stderr

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
2+
--> $DIR/orphan-check-opaque-types-not-covering.rs:17:6
3+
|
4+
LL | impl<T> foreign::Trait0<Local, T, ()> for Identity<T> {}
5+
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
6+
|
7+
= note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
8+
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
9+
10+
error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
11+
--> $DIR/orphan-check-opaque-types-not-covering.rs:26:6
12+
|
13+
LL | impl<T> foreign::Trait1<Local, T> for Opaque<T> {}
14+
| ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`)
15+
|
16+
= note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local, and no uncovered type parameters appear before that first local type
17+
= note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait<T1, ..., Tn> for T0`, where `T0` is the first and `Tn` is the last
18+
19+
error: aborting due to 2 previous errors
20+
21+
For more information about this error, try `rustc --explain E0210`.

‎tests/ui/coherence/orphan-check-opaque-types-not-covering.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Opaque types never cover type parameters.
2+
3+
//@ revisions: classic next
4+
//@[next] compile-flags: -Znext-solver
5+
6+
//@ aux-crate:foreign=parametrized-trait.rs
7+
//@ edition:2021
8+
9+
#![feature(type_alias_impl_trait)]
10+
11+
type Identity<T> = impl Sized;
12+
13+
fn define_identity<T>(x: T) -> Identity<T> {
14+
x
15+
}
16+
17+
impl<T> foreign::Trait0<Local, T, ()> for Identity<T> {}
18+
//~^ ERROR type parameter `T` must be covered by another type
19+
20+
type Opaque<T> = impl Sized;
21+
22+
fn define_local<T>() -> Opaque<T> {
23+
Local
24+
}
25+
26+
impl<T> foreign::Trait1<Local, T> for Opaque<T> {}
27+
//~^ ERROR type parameter `T` must be covered by another type
28+
29+
struct Local;
30+
31+
fn main() {}

‎tests/ui/type-alias-impl-trait/coherence.stderr renamed to ‎tests/ui/type-alias-impl-trait/coherence.classic.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
2-
--> $DIR/coherence.rs:14:1
2+
--> $DIR/coherence.rs:16:1
33
|
44
LL | impl foreign_crate::ForeignTrait for AliasOfForeignType<()> {}
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^----------------------

‎tests/ui/type-alias-impl-trait/coherence.next.stderr

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
2+
--> $DIR/coherence.rs:16:1
3+
|
4+
LL | impl foreign_crate::ForeignTrait for AliasOfForeignType<()> {}
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use only types from inside the current crate
6+
|
7+
= note: define and implement a trait or new type instead
8+
9+
error: aborting due to 1 previous error
10+
11+
For more information about this error, try `rustc --explain E0117`.

‎tests/ui/type-alias-impl-trait/coherence.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
//@ aux-build:foreign-crate.rs
2+
//@ revisions: classic next
3+
//@[next] compile-flags: -Znext-solver
24
#![feature(type_alias_impl_trait)]
35

46
extern crate foreign_crate;

0 commit comments

Comments
 (0)