From 58781edc5459d9835b80d819d0469a23bdfc0026 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 12 Apr 2022 16:02:57 +0200 Subject: [PATCH] update coherence docs, fix opaque type + generator ice --- compiler/rustc_middle/src/traits/mod.rs | 3 ++ .../src/traits/coherence.rs | 27 ++++++++++------- .../src/traits/select/mod.rs | 30 +++++++++++-------- .../ui/coherence/coherence-with-closure.rs | 15 ++++++++++ .../coherence/coherence-with-closure.stderr | 24 +++++++++++++++ .../ui/coherence/coherence-with-generator.rs | 19 ++++++++++++ .../coherence/coherence-with-generator.stderr | 24 +++++++++++++++ 7 files changed, 120 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/coherence/coherence-with-closure.rs create mode 100644 src/test/ui/coherence/coherence-with-closure.stderr create mode 100644 src/test/ui/coherence/coherence-with-generator.rs create mode 100644 src/test/ui/coherence/coherence-with-generator.stderr diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 7c3d08b26bf54..3f4c43dceff0d 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -60,6 +60,9 @@ pub enum Reveal { /// let <() as Assoc>::Output = true; /// } /// ``` + /// + /// We also do not reveal the hidden type of opaque types during + /// type-checking. UserFacing, /// At codegen time, all monomorphic projections will succeed. diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 40c3f4047ae6e..37e2eeca86fd2 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -645,7 +645,7 @@ fn orphan_check_trait_ref<'tcx>( .substs .types() .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) - .find(|ty| ty_is_local_constructor(*ty, in_crate)); + .find(|&ty| ty_is_local_constructor(tcx, ty, in_crate)); debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type); @@ -677,7 +677,7 @@ fn contained_non_local_types<'tcx>( ty: Ty<'tcx>, in_crate: InCrate, ) -> Vec> { - if ty_is_local_constructor(ty, in_crate) { + if ty_is_local_constructor(tcx, ty, in_crate) { Vec::new() } else { match fundamental_ty_inner_tys(tcx, ty) { @@ -730,7 +730,7 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool { } } -fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool { +fn ty_is_local_constructor(tcx: TyCtxt<'_>, ty: Ty<'_>, in_crate: InCrate) -> bool { debug!("ty_is_local_constructor({:?})", ty); match *ty.kind() { @@ -789,11 +789,6 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool { false } - ty::Closure(..) => { - // Similar to the `Opaque` case (#83613). - false - } - ty::Dynamic(ref tt, ..) => { if let Some(principal) = tt.principal() { def_id_is_local(principal.def_id(), in_crate) @@ -804,8 +799,20 @@ fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool { ty::Error(_) => true, - ty::Generator(..) | ty::GeneratorWitness(..) => { - bug!("ty_is_local invoked on unexpected type: {:?}", ty) + // These variants should never appear during coherence checking because they + // cannot be named directly. + // + // They could be indirectly used through an opaque type. While using opaque types + // in impls causes an error, this path can still be hit afterwards. + // + // See `test/ui/coherence/coherence-with-closure.rs` for an example where this + // could happens. + ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { + tcx.sess.delay_span_bug( + DUMMY_SP, + format!("ty_is_local invoked on closure or generator: {:?}", ty), + ); + true } } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index e69bf6eca90ef..379f5a120406b 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -103,22 +103,31 @@ pub struct SelectionContext<'cx, 'tcx> { /// require themselves. freshener: TypeFreshener<'cx, 'tcx>, - /// If `true`, indicates that the evaluation should be conservative - /// and consider the possibility of types outside this crate. + /// During coherence we have to assume that other crates may add + /// additional impls which we currently don't know about. + /// + /// To deal with this evaluation should be conservative + /// and consider the possibility of impls from outside this crate. /// This comes up primarily when resolving ambiguity. Imagine /// there is some trait reference `$0: Bar` where `$0` is an /// inference variable. If `intercrate` is true, then we can never /// say for sure that this reference is not implemented, even if /// there are *no impls at all for `Bar`*, because `$0` could be /// bound to some type that in a downstream crate that implements - /// `Bar`. This is the suitable mode for coherence. Elsewhere, - /// though, we set this to false, because we are only interested - /// in types that the user could actually have written --- in - /// other words, we consider `$0: Bar` to be unimplemented if + /// `Bar`. + /// + /// Outside of coherence we set this to false because we are only + /// interested in types that the user could actually have written. + /// In other words, we consider `$0: Bar` to be unimplemented if /// there is no type that the user could *actually name* that /// would satisfy it. This avoids crippling inference, basically. intercrate: bool, - + /// If `intercrate` is set, we remember predicates which were + /// considered ambiguous because of impls potentially added in other crates. + /// This is used in coherence to give improved diagnostics. + /// We don't do his until we detect a coherence error because it can + /// lead to false overflow results (#47139) and because always + /// computing it may negatively impact performance. intercrate_ambiguity_causes: Option>, /// The mode that trait queries run in, which informs our error handling @@ -240,11 +249,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } - /// Enables tracking of intercrate ambiguity causes. These are - /// used in coherence to give improved diagnostics. We don't do - /// this until we detect a coherence error because it can lead to - /// false overflow results (#47139) and because it costs - /// computation time. + /// Enables tracking of intercrate ambiguity causes. See + /// the documentation of [`Self::intercrate_ambiguity_causes`] for more. pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) { assert!(self.intercrate); assert!(self.intercrate_ambiguity_causes.is_none()); diff --git a/src/test/ui/coherence/coherence-with-closure.rs b/src/test/ui/coherence/coherence-with-closure.rs new file mode 100644 index 0000000000000..6e3281d8508ab --- /dev/null +++ b/src/test/ui/coherence/coherence-with-closure.rs @@ -0,0 +1,15 @@ +// Test that encountering closures during coherence does not cause issues. +#![feature(type_alias_impl_trait)] +type OpaqueClosure = impl Sized; +fn defining_use() -> OpaqueClosure { + || () +} + +struct Wrapper(T); +trait Trait {} +impl Trait for Wrapper {} +//~^ ERROR cannot implement trait on type alias impl trait +impl Trait for Wrapper {} +//~^ ERROR conflicting implementations of trait `Trait` for type `Wrapper` + +fn main() {} diff --git a/src/test/ui/coherence/coherence-with-closure.stderr b/src/test/ui/coherence/coherence-with-closure.stderr new file mode 100644 index 0000000000000..20b986cee6913 --- /dev/null +++ b/src/test/ui/coherence/coherence-with-closure.stderr @@ -0,0 +1,24 @@ +error: cannot implement trait on type alias impl trait + --> $DIR/coherence-with-closure.rs:10:24 + | +LL | impl Trait for Wrapper {} + | ^^^^^^^^^^^^^ + | +note: type alias impl trait defined here + --> $DIR/coherence-with-closure.rs:3:22 + | +LL | type OpaqueClosure = impl Sized; + | ^^^^^^^^^^ + +error[E0119]: conflicting implementations of trait `Trait` for type `Wrapper` + --> $DIR/coherence-with-closure.rs:12:1 + | +LL | impl Trait for Wrapper {} + | ------------------------------------- first implementation here +LL | +LL | impl Trait for Wrapper {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Wrapper` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0119`. diff --git a/src/test/ui/coherence/coherence-with-generator.rs b/src/test/ui/coherence/coherence-with-generator.rs new file mode 100644 index 0000000000000..d34c391db9fb0 --- /dev/null +++ b/src/test/ui/coherence/coherence-with-generator.rs @@ -0,0 +1,19 @@ +// Test that encountering closures during coherence does not cause issues. +#![feature(type_alias_impl_trait, generators)] +type OpaqueGenerator = impl Sized; +fn defining_use() -> OpaqueGenerator { + || { + for i in 0..10 { + yield i; + } + } +} + +struct Wrapper(T); +trait Trait {} +impl Trait for Wrapper {} +//~^ ERROR cannot implement trait on type alias impl trait +impl Trait for Wrapper {} +//~^ ERROR conflicting implementations of trait `Trait` for type `Wrapper` + +fn main() {} diff --git a/src/test/ui/coherence/coherence-with-generator.stderr b/src/test/ui/coherence/coherence-with-generator.stderr new file mode 100644 index 0000000000000..249ad3cb9ec7f --- /dev/null +++ b/src/test/ui/coherence/coherence-with-generator.stderr @@ -0,0 +1,24 @@ +error: cannot implement trait on type alias impl trait + --> $DIR/coherence-with-generator.rs:14:24 + | +LL | impl Trait for Wrapper {} + | ^^^^^^^^^^^^^^^ + | +note: type alias impl trait defined here + --> $DIR/coherence-with-generator.rs:3:24 + | +LL | type OpaqueGenerator = impl Sized; + | ^^^^^^^^^^ + +error[E0119]: conflicting implementations of trait `Trait` for type `Wrapper` + --> $DIR/coherence-with-generator.rs:16:1 + | +LL | impl Trait for Wrapper {} + | --------------------------------------- first implementation here +LL | +LL | impl Trait for Wrapper {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Wrapper` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0119`.