about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Goulet <michael@errs.io>2023-07-30 21:46:04 +0000
committerMichael Goulet <michael@errs.io>2023-08-15 03:44:21 +0000
commit0e20155662e5513e0bcdb1300c81458c16b7cca9 (patch)
tree977a25820bd83e3bd13456f0556d1fde6ae2def4
parentca49a37390ed4fdead6432c322ab246b63306705 (diff)
downloadrust-0e20155662e5513e0bcdb1300c81458c16b7cca9.tar.gz
rust-0e20155662e5513e0bcdb1300c81458c16b7cca9.zip
more nits
-rw-r--r--compiler/rustc_lint_defs/src/builtin.rs41
-rw-r--r--compiler/rustc_trait_selection/src/traits/coherence.rs33
-rw-r--r--compiler/rustc_trait_selection/src/traits/select/mod.rs33
-rw-r--r--tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs2
-rw-r--r--tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr5
5 files changed, 65 insertions, 49 deletions
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
index cd5269dab9b..96c31a90da8 100644
--- a/compiler/rustc_lint_defs/src/builtin.rs
+++ b/compiler/rustc_lint_defs/src/builtin.rs
@@ -4433,42 +4433,25 @@ declare_lint! {
     /// ```rust,compile_fail
     /// #![deny(coinductive_overlap_in_coherence)]
     ///
-    /// use std::borrow::Borrow;
-    /// use std::cmp::Ordering;
-    /// use std::marker::PhantomData;
+    /// trait CyclicTrait {}
+    /// impl<T: CyclicTrait> CyclicTrait for T {}
     ///
-    /// #[derive(PartialEq, Default)]
-    /// pub(crate) struct Interval<T>(T);
-    ///
-    /// impl<T, Q> PartialEq<Q> for Interval<T>
-    /// where
-    ///     Q: PartialOrd,
-    /// {
-    ///     fn eq(&self, other: &Q) -> bool {
-    ///         todo!()
-    ///     }
-    /// }
-    ///
-    /// impl<T, Q> PartialOrd<Q> for Interval<T>
-    /// where
-    ///     Q: PartialOrd,
-    /// {
-    ///     fn partial_cmp(&self, other: &Q) -> Option<Ordering> {
-    ///         todo!()
-    ///     }
-    /// }
+    /// trait Trait {}
+    /// impl<T: CyclicTrait> Trait for T {}
+    /// // conflicting impl with the above
+    /// impl Trait for u8 {}
     /// ```
     ///
     /// {{produces}}
     ///
     /// ### Explanation
     ///
-    /// The manual impl of `PartialEq` impl overlaps with the `derive`, since
-    /// if we replace `Q = Interval<T>`, then the second impl leads to a cycle:
-    /// `PartialOrd for Interval<T> where Interval<T>: PartialOrd`. This cycle
-    /// currently causes the compiler to consider `Interval<T>: PartialOrd` to not
-    /// hold, causing the two implementations to be disjoint. This will change in
-    /// a future release.
+    /// We have two choices for impl which satisfy `u8: Trait`: the blanket impl
+    /// for generic `T`, and the direct impl for `u8`. These two impls nominally
+    /// overlap, since we can infer `T = u8` in the former impl, but since the where
+    /// clause `u8: CyclicTrait` would end up resulting in a cycle (since it depends
+    /// on itself), the blanket impl is not considered to hold for `u8`. This will
+    /// change in a future release.
     pub COINDUCTIVE_OVERLAP_IN_COHERENCE,
     Warn,
     "impls that are not considered to overlap may be considered to \
diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs
index 19a122e8b90..5746781ae35 100644
--- a/compiler/rustc_trait_selection/src/traits/coherence.rs
+++ b/compiler/rustc_trait_selection/src/traits/coherence.rs
@@ -231,10 +231,29 @@ fn overlap<'tcx>(
                         COINDUCTIVE_OVERLAP_IN_COHERENCE,
                         infcx.tcx.local_def_id_to_hir_id(first_local_impl),
                         infcx.tcx.def_span(first_local_impl),
-                        "impls that are not considered to overlap may be considered to \
-                overlap in the future",
+                        format!(
+                            "implementations {} will conflict in the future",
+                            match impl1_header.trait_ref {
+                                Some(trait_ref) => {
+                                    let trait_ref = infcx.resolve_vars_if_possible(trait_ref);
+                                    format!(
+                                        "of `{}` for `{}`",
+                                        trait_ref.print_only_trait_path(),
+                                        trait_ref.self_ty()
+                                    )
+                                }
+                                None => format!(
+                                    "for `{}`",
+                                    infcx.resolve_vars_if_possible(impl1_header.self_ty)
+                                ),
+                            },
+                        ),
                         |lint| {
-                            lint.span_label(
+                            lint.note(
+                                "impls that are not considered to overlap may be considered to \
+                                overlap in the future",
+                            )
+                            .span_label(
                                 infcx.tcx.def_span(impl1_header.impl_def_id),
                                 "the first impl is here",
                             )
@@ -245,8 +264,12 @@ fn overlap<'tcx>(
                             if !failing_obligation.cause.span.is_dummy() {
                                 lint.span_label(
                                     failing_obligation.cause.span,
-                                    "this where-clause causes a cycle, but it may be treated \
-                            as possibly holding in the future, causing the impls to overlap",
+                                    format!(
+                                        "`{}` may be considered to hold in future releases, \
+                                        causing the impls to overlap",
+                                        infcx
+                                            .resolve_vars_if_possible(failing_obligation.predicate)
+                                    ),
                                 );
                             }
                             lint
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index 6ca818b79cf..19385e2d7f2 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -202,10 +202,25 @@ enum BuiltinImplConditions<'tcx> {
 
 #[derive(Copy, Clone)]
 pub enum TreatInductiveCycleAs {
+    /// This is the previous behavior, where `Recur` represents an inductive
+    /// cycle that is known not to hold. This is not forwards-compatible with
+    /// coinduction, and will be deprecated. This is the default behavior
+    /// of the old trait solver due to back-compat reasons.
     Recur,
+    /// This is the behavior of the new trait solver, where inductive cycles
+    /// are treated as ambiguous and possibly holding.
     Ambig,
 }
 
+impl From<TreatInductiveCycleAs> for EvaluationResult {
+    fn from(treat: TreatInductiveCycleAs) -> EvaluationResult {
+        match treat {
+            TreatInductiveCycleAs::Ambig => EvaluatedToUnknown,
+            TreatInductiveCycleAs::Recur => EvaluatedToRecur,
+        }
+    }
+}
+
 impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> {
         SelectionContext {
@@ -223,6 +238,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         treat_inductive_cycle: TreatInductiveCycleAs,
         f: impl FnOnce(&mut Self) -> T,
     ) -> T {
+        // Should be executed in a context where caching is disabled,
+        // otherwise the cache is poisoned with the temporary result.
+        assert!(self.is_intercrate());
         let treat_inductive_cycle =
             std::mem::replace(&mut self.treat_inductive_cycle, treat_inductive_cycle);
         let value = f(self);
@@ -741,10 +759,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                 stack.update_reached_depth(stack_arg.1);
                                 return Ok(EvaluatedToOk);
                             } else {
-                                match self.treat_inductive_cycle {
-                                    TreatInductiveCycleAs::Ambig => return Ok(EvaluatedToUnknown),
-                                    TreatInductiveCycleAs::Recur => return Ok(EvaluatedToRecur),
-                                }
+                                return Ok(self.treat_inductive_cycle.into());
                             }
                         }
                         return Ok(EvaluatedToOk);
@@ -862,10 +877,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                             }
                         }
                         ProjectAndUnifyResult::FailedNormalization => Ok(EvaluatedToAmbig),
-                        ProjectAndUnifyResult::Recursive => match self.treat_inductive_cycle {
-                            TreatInductiveCycleAs::Ambig => return Ok(EvaluatedToUnknown),
-                            TreatInductiveCycleAs::Recur => return Ok(EvaluatedToRecur),
-                        },
+                        ProjectAndUnifyResult::Recursive => Ok(self.treat_inductive_cycle.into()),
                         ProjectAndUnifyResult::MismatchedProjectionTypes(_) => Ok(EvaluatedToErr),
                     }
                 }
@@ -1179,10 +1191,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 Some(EvaluatedToOk)
             } else {
                 debug!("evaluate_stack --> recursive, inductive");
-                match self.treat_inductive_cycle {
-                    TreatInductiveCycleAs::Ambig => Some(EvaluatedToUnknown),
-                    TreatInductiveCycleAs::Recur => Some(EvaluatedToRecur),
-                }
+                Some(self.treat_inductive_cycle.into())
             }
         } else {
             None
diff --git a/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs b/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs
index 268fe56368c..01f7d6ce901 100644
--- a/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs
+++ b/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs
@@ -11,7 +11,7 @@ pub(crate) struct Interval<T>(PhantomData<T>);
 // `Interval<?1>: PartialOrd<Interval<?1>>` candidate which results
 // in a - currently inductive - cycle.
 impl<T, Q> PartialEq<Q> for Interval<T>
-//~^ ERROR impls that are not considered to overlap may be considered to overlap in the future
+//~^ ERROR implementations of `PartialEq<Interval<_>>` for `Interval<_>` will conflict in the future
 //~| WARN this was previously accepted by the compiler but is being phased out
 where
     T: Borrow<Q>,
diff --git a/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr b/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr
index fd6193f4b74..f315ba82130 100644
--- a/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr
+++ b/tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr
@@ -1,4 +1,4 @@
-error: impls that are not considered to overlap may be considered to overlap in the future
+error: implementations of `PartialEq<Interval<_>>` for `Interval<_>` will conflict in the future
   --> $DIR/warn-when-cycle-is-error-in-coherence.rs:13:1
    |
 LL | #[derive(PartialEq, Default)]
@@ -8,10 +8,11 @@ LL | impl<T, Q> PartialEq<Q> for Interval<T>
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the first impl is here
 ...
 LL |     Q: ?Sized + PartialOrd,
-   |                 ---------- this where-clause causes a cycle, but it may be treated as possibly holding in the future, causing the impls to overlap
+   |                 ---------- `Interval<_>: PartialOrd` may be considered to hold in future releases, causing the impls to overlap
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #114040 <https://github.com/rust-lang/rust/issues/114040>
+   = note: impls that are not considered to overlap may be considered to overlap in the future
 note: the lint level is defined here
   --> $DIR/warn-when-cycle-is-error-in-coherence.rs:1:9
    |