Skip to content

Commit 9c8e46d

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #125871 - fmease:fix-orphanck-opaques, r=lcnr
Orphanck[old solver]: Consider opaque types to never cover type parameters This fixes an oversight of mine in #117164. The change itself has already been FCP'ed. This only affects the old solver, the next solver already correctly rejects the added test since #117164. r? ``@lcnr``
2 parents fcc0b64 + 8746703 commit 9c8e46d

7 files changed

+114
-36
lines changed

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

+24-35
Original file line numberDiff line numberDiff line change
@@ -924,11 +924,12 @@ where
924924
}
925925
}
926926

927-
ty::Alias(kind @ (ty::Projection | ty::Inherent | ty::Weak), ..) => {
928-
if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) {
929-
bug!("unexpected ty param in alias ty");
930-
}
931-
927+
// A rigid alias may normalize to anything.
928+
// * If it references an infer var, placeholder or bound ty, it may
929+
// normalize to that, so we have to treat it as an uncovered ty param.
930+
// * Otherwise it may normalize to any non-type-generic type
931+
// be it local or non-local.
932+
ty::Alias(kind, _) => {
932933
if ty.has_type_flags(
933934
ty::TypeFlags::HAS_TY_PLACEHOLDER
934935
| ty::TypeFlags::HAS_TY_BOUND
@@ -948,7 +949,24 @@ where
948949
}
949950
}
950951
} else {
951-
ControlFlow::Continue(())
952+
// Regarding *opaque types* specifically, we choose to treat them as non-local,
953+
// even those that appear within the same crate. This seems somewhat surprising
954+
// at first, but makes sense when you consider that opaque types are supposed
955+
// to hide the underlying type *within the same crate*. When an opaque type is
956+
// used from outside the module where it is declared, it should be impossible to
957+
// observe anything about it other than the traits that it implements.
958+
//
959+
// The alternative would be to look at the underlying type to determine whether
960+
// or not the opaque type itself should be considered local.
961+
//
962+
// However, this could make it a breaking change to switch the underlying hidden
963+
// type from a local type to a remote type. This would violate the rule that
964+
// opaque types should be completely opaque apart from the traits that they
965+
// implement, so we don't use this behavior.
966+
// Addendum: Moreover, revealing the underlying type is likely to cause cycle
967+
// errors as we rely on coherence / the specialization graph during typeck.
968+
969+
self.found_non_local_ty(ty)
952970
}
953971
}
954972

@@ -990,35 +1008,6 @@ where
9901008
// auto trait impl applies. There will never be multiple impls, so we can just
9911009
// act as if it were a local type here.
9921010
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-
}
10221011
};
10231012
// A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so
10241013
// 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

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
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+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^----------------------
6+
| | |
7+
| | `AliasOfForeignType<()>` is not defined in the current crate
8+
| impl doesn't use only types from inside the current crate
9+
|
10+
= note: define and implement a trait or new type instead
11+
12+
error: aborting due to 1 previous error
13+
14+
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)