From 874670399c43c58a6e21d46662979fb4fb8a0b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sat, 1 Jun 2024 23:44:59 +0200 Subject: [PATCH] Orphanck: Consider opaque types to never cover type parameters --- .../src/traits/coherence.rs | 59 ++++++++----------- ...k-opaque-types-not-covering.classic.stderr | 21 +++++++ ...heck-opaque-types-not-covering.next.stderr | 21 +++++++ .../orphan-check-opaque-types-not-covering.rs | 31 ++++++++++ ...erence.stderr => coherence.classic.stderr} | 2 +- .../coherence.next.stderr | 14 +++++ tests/ui/type-alias-impl-trait/coherence.rs | 2 + 7 files changed, 114 insertions(+), 36 deletions(-) create mode 100644 tests/ui/coherence/orphan-check-opaque-types-not-covering.classic.stderr create mode 100644 tests/ui/coherence/orphan-check-opaque-types-not-covering.next.stderr create mode 100644 tests/ui/coherence/orphan-check-opaque-types-not-covering.rs rename tests/ui/type-alias-impl-trait/{coherence.stderr => coherence.classic.stderr} (95%) create mode 100644 tests/ui/type-alias-impl-trait/coherence.next.stderr diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index ebdb032dc0e25..c81f5ac6e6efb 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -924,11 +924,12 @@ where } } - ty::Alias(kind @ (ty::Projection | ty::Inherent | ty::Weak), ..) => { - if ty.has_type_flags(ty::TypeFlags::HAS_TY_PARAM) { - bug!("unexpected ty param in alias ty"); - } - + // A rigid alias may normalize to anything. + // * If it references an infer var, placeholder or bound ty, it may + // normalize to that, so we have to treat it as an uncovered ty param. + // * Otherwise it may normalize to any non-type-generic type + // be it local or non-local. + ty::Alias(kind, _) => { if ty.has_type_flags( ty::TypeFlags::HAS_TY_PLACEHOLDER | ty::TypeFlags::HAS_TY_BOUND @@ -948,7 +949,24 @@ where } } } else { - ControlFlow::Continue(()) + // Regarding *opaque types* specifically, we choose to treat them as non-local, + // even those that appear within the same crate. This seems somewhat surprising + // 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 anything 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 hidden + // 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. + // Addendum: Moreover, revealing the underlying type is likely to cause cycle + // errors as we rely on coherence / the specialization graph during typeck. + + self.found_non_local_ty(ty) } } @@ -990,35 +1008,6 @@ where // auto trait impl applies. There will never be multiple impls, so we can just // act as if it were a local type here. ty::CoroutineWitness(..) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)), - ty::Alias(ty::Opaque, ..) => { - // This merits some explanation. - // Normally, opaque types are not involved 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 - // the type of a field) when checking for auto trait or `Sized` - // impls. 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 surprising 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 - // anything 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. - self.found_non_local_ty(ty) - } }; // A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so // the first type we visit is always the self type. diff --git a/tests/ui/coherence/orphan-check-opaque-types-not-covering.classic.stderr b/tests/ui/coherence/orphan-check-opaque-types-not-covering.classic.stderr new file mode 100644 index 0000000000000..44f76f321cf10 --- /dev/null +++ b/tests/ui/coherence/orphan-check-opaque-types-not-covering.classic.stderr @@ -0,0 +1,21 @@ +error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + --> $DIR/orphan-check-opaque-types-not-covering.rs:17:6 + | +LL | impl foreign::Trait0 for Identity {} + | ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + | + = 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 + = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait for T0`, where `T0` is the first and `Tn` is the last + +error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + --> $DIR/orphan-check-opaque-types-not-covering.rs:26:6 + | +LL | impl foreign::Trait1 for Opaque {} + | ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + | + = 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 + = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait for T0`, where `T0` is the first and `Tn` is the last + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/tests/ui/coherence/orphan-check-opaque-types-not-covering.next.stderr b/tests/ui/coherence/orphan-check-opaque-types-not-covering.next.stderr new file mode 100644 index 0000000000000..44f76f321cf10 --- /dev/null +++ b/tests/ui/coherence/orphan-check-opaque-types-not-covering.next.stderr @@ -0,0 +1,21 @@ +error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + --> $DIR/orphan-check-opaque-types-not-covering.rs:17:6 + | +LL | impl foreign::Trait0 for Identity {} + | ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + | + = 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 + = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait for T0`, where `T0` is the first and `Tn` is the last + +error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + --> $DIR/orphan-check-opaque-types-not-covering.rs:26:6 + | +LL | impl foreign::Trait1 for Opaque {} + | ^ type parameter `T` must be covered by another type when it appears before the first local type (`Local`) + | + = 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 + = note: in this case, 'before' refers to the following order: `impl<..> ForeignTrait for T0`, where `T0` is the first and `Tn` is the last + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/tests/ui/coherence/orphan-check-opaque-types-not-covering.rs b/tests/ui/coherence/orphan-check-opaque-types-not-covering.rs new file mode 100644 index 0000000000000..8dc02b081c51d --- /dev/null +++ b/tests/ui/coherence/orphan-check-opaque-types-not-covering.rs @@ -0,0 +1,31 @@ +// Opaque types never cover type parameters. + +//@ revisions: classic next +//@[next] compile-flags: -Znext-solver + +//@ aux-crate:foreign=parametrized-trait.rs +//@ edition:2021 + +#![feature(type_alias_impl_trait)] + +type Identity = impl Sized; + +fn define_identity(x: T) -> Identity { + x +} + +impl foreign::Trait0 for Identity {} +//~^ ERROR type parameter `T` must be covered by another type + +type Opaque = impl Sized; + +fn define_local() -> Opaque { + Local +} + +impl foreign::Trait1 for Opaque {} +//~^ ERROR type parameter `T` must be covered by another type + +struct Local; + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/coherence.stderr b/tests/ui/type-alias-impl-trait/coherence.classic.stderr similarity index 95% rename from tests/ui/type-alias-impl-trait/coherence.stderr rename to tests/ui/type-alias-impl-trait/coherence.classic.stderr index 266a532a1db19..ff059bc5806de 100644 --- a/tests/ui/type-alias-impl-trait/coherence.stderr +++ b/tests/ui/type-alias-impl-trait/coherence.classic.stderr @@ -1,5 +1,5 @@ error[E0117]: only traits defined in the current crate can be implemented for arbitrary types - --> $DIR/coherence.rs:14:1 + --> $DIR/coherence.rs:16:1 | LL | impl foreign_crate::ForeignTrait for AliasOfForeignType<()> {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^---------------------- diff --git a/tests/ui/type-alias-impl-trait/coherence.next.stderr b/tests/ui/type-alias-impl-trait/coherence.next.stderr new file mode 100644 index 0000000000000..dab2786c1f0fb --- /dev/null +++ b/tests/ui/type-alias-impl-trait/coherence.next.stderr @@ -0,0 +1,14 @@ +error[E0117]: only traits defined in the current crate can be implemented for arbitrary types + --> $DIR/coherence.rs:16:1 + | +LL | impl foreign_crate::ForeignTrait for AliasOfForeignType<()> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^---------------------- + | | | + | | `AliasOfForeignType<()>` is not defined in the current crate + | impl doesn't use only types from inside the current crate + | + = note: define and implement a trait or new type instead + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0117`. diff --git a/tests/ui/type-alias-impl-trait/coherence.rs b/tests/ui/type-alias-impl-trait/coherence.rs index 641c0fac17a64..760e5210c5b7a 100644 --- a/tests/ui/type-alias-impl-trait/coherence.rs +++ b/tests/ui/type-alias-impl-trait/coherence.rs @@ -1,4 +1,6 @@ //@ aux-build:foreign-crate.rs +//@ revisions: classic next +//@[next] compile-flags: -Znext-solver #![feature(type_alias_impl_trait)] extern crate foreign_crate;