From 0e20155662e5513e0bcdb1300c81458c16b7cca9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 30 Jul 2023 21:46:04 +0000 Subject: [PATCH] more nits --- compiler/rustc_lint_defs/src/builtin.rs | 41 ++++++------------- .../src/traits/coherence.rs | 33 ++++++++++++--- .../src/traits/select/mod.rs | 33 +++++++++------ .../warn-when-cycle-is-error-in-coherence.rs | 2 +- ...rn-when-cycle-is-error-in-coherence.stderr | 5 ++- 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 CyclicTrait for T {} /// - /// #[derive(PartialEq, Default)] - /// pub(crate) struct Interval(T); - /// - /// impl PartialEq for Interval - /// where - /// Q: PartialOrd, - /// { - /// fn eq(&self, other: &Q) -> bool { - /// todo!() - /// } - /// } - /// - /// impl PartialOrd for Interval - /// where - /// Q: PartialOrd, - /// { - /// fn partial_cmp(&self, other: &Q) -> Option { - /// todo!() - /// } - /// } + /// trait Trait {} + /// impl 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`, then the second impl leads to a cycle: - /// `PartialOrd for Interval where Interval: PartialOrd`. This cycle - /// currently causes the compiler to consider `Interval: 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 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(PhantomData); // `Interval: PartialOrd>` candidate which results // in a - currently inductive - cycle. impl PartialEq for Interval -//~^ ERROR impls that are not considered to overlap may be considered to overlap in the future +//~^ ERROR implementations of `PartialEq>` for `Interval<_>` will conflict in the future //~| WARN this was previously accepted by the compiler but is being phased out where T: Borrow, 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>` 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 PartialEq for Interval | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 + = 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 |