1
Fork 0

more nits

This commit is contained in:
Michael Goulet 2023-07-30 21:46:04 +00:00
parent ca49a37390
commit 0e20155662
5 changed files with 65 additions and 49 deletions

View file

@ -4433,42 +4433,25 @@ declare_lint! {
/// ```rust,compile_fail /// ```rust,compile_fail
/// #![deny(coinductive_overlap_in_coherence)] /// #![deny(coinductive_overlap_in_coherence)]
/// ///
/// use std::borrow::Borrow; /// trait CyclicTrait {}
/// use std::cmp::Ordering; /// impl<T: CyclicTrait> CyclicTrait for T {}
/// use std::marker::PhantomData;
/// ///
/// #[derive(PartialEq, Default)] /// trait Trait {}
/// pub(crate) struct Interval<T>(T); /// impl<T: CyclicTrait> Trait for T {}
/// /// // conflicting impl with the above
/// impl<T, Q> PartialEq<Q> for Interval<T> /// impl Trait for u8 {}
/// 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!()
/// }
/// }
/// ``` /// ```
/// ///
/// {{produces}} /// {{produces}}
/// ///
/// ### Explanation /// ### Explanation
/// ///
/// The manual impl of `PartialEq` impl overlaps with the `derive`, since /// We have two choices for impl which satisfy `u8: Trait`: the blanket impl
/// if we replace `Q = Interval<T>`, then the second impl leads to a cycle: /// for generic `T`, and the direct impl for `u8`. These two impls nominally
/// `PartialOrd for Interval<T> where Interval<T>: PartialOrd`. This cycle /// overlap, since we can infer `T = u8` in the former impl, but since the where
/// currently causes the compiler to consider `Interval<T>: PartialOrd` to not /// clause `u8: CyclicTrait` would end up resulting in a cycle (since it depends
/// hold, causing the two implementations to be disjoint. This will change in /// on itself), the blanket impl is not considered to hold for `u8`. This will
/// a future release. /// change in a future release.
pub COINDUCTIVE_OVERLAP_IN_COHERENCE, pub COINDUCTIVE_OVERLAP_IN_COHERENCE,
Warn, Warn,
"impls that are not considered to overlap may be considered to \ "impls that are not considered to overlap may be considered to \

View file

@ -231,10 +231,29 @@ fn overlap<'tcx>(
COINDUCTIVE_OVERLAP_IN_COHERENCE, COINDUCTIVE_OVERLAP_IN_COHERENCE,
infcx.tcx.local_def_id_to_hir_id(first_local_impl), infcx.tcx.local_def_id_to_hir_id(first_local_impl),
infcx.tcx.def_span(first_local_impl), infcx.tcx.def_span(first_local_impl),
"impls that are not considered to overlap may be considered to \ format!(
overlap in the future", "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| {
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), infcx.tcx.def_span(impl1_header.impl_def_id),
"the first impl is here", "the first impl is here",
) )
@ -245,8 +264,12 @@ fn overlap<'tcx>(
if !failing_obligation.cause.span.is_dummy() { if !failing_obligation.cause.span.is_dummy() {
lint.span_label( lint.span_label(
failing_obligation.cause.span, failing_obligation.cause.span,
"this where-clause causes a cycle, but it may be treated \ format!(
as possibly holding in the future, causing the impls to overlap", "`{}` may be considered to hold in future releases, \
causing the impls to overlap",
infcx
.resolve_vars_if_possible(failing_obligation.predicate)
),
); );
} }
lint lint

View file

@ -202,10 +202,25 @@ enum BuiltinImplConditions<'tcx> {
#[derive(Copy, Clone)] #[derive(Copy, Clone)]
pub enum TreatInductiveCycleAs { 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, Recur,
/// This is the behavior of the new trait solver, where inductive cycles
/// are treated as ambiguous and possibly holding.
Ambig, 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> { impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> { pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> {
SelectionContext { SelectionContext {
@ -223,6 +238,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
treat_inductive_cycle: TreatInductiveCycleAs, treat_inductive_cycle: TreatInductiveCycleAs,
f: impl FnOnce(&mut Self) -> T, f: impl FnOnce(&mut Self) -> T,
) -> 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 = let treat_inductive_cycle =
std::mem::replace(&mut self.treat_inductive_cycle, treat_inductive_cycle); std::mem::replace(&mut self.treat_inductive_cycle, treat_inductive_cycle);
let value = f(self); let value = f(self);
@ -741,10 +759,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
stack.update_reached_depth(stack_arg.1); stack.update_reached_depth(stack_arg.1);
return Ok(EvaluatedToOk); return Ok(EvaluatedToOk);
} else { } else {
match self.treat_inductive_cycle { return Ok(self.treat_inductive_cycle.into());
TreatInductiveCycleAs::Ambig => return Ok(EvaluatedToUnknown),
TreatInductiveCycleAs::Recur => return Ok(EvaluatedToRecur),
}
} }
} }
return Ok(EvaluatedToOk); return Ok(EvaluatedToOk);
@ -862,10 +877,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
} }
} }
ProjectAndUnifyResult::FailedNormalization => Ok(EvaluatedToAmbig), ProjectAndUnifyResult::FailedNormalization => Ok(EvaluatedToAmbig),
ProjectAndUnifyResult::Recursive => match self.treat_inductive_cycle { ProjectAndUnifyResult::Recursive => Ok(self.treat_inductive_cycle.into()),
TreatInductiveCycleAs::Ambig => return Ok(EvaluatedToUnknown),
TreatInductiveCycleAs::Recur => return Ok(EvaluatedToRecur),
},
ProjectAndUnifyResult::MismatchedProjectionTypes(_) => Ok(EvaluatedToErr), ProjectAndUnifyResult::MismatchedProjectionTypes(_) => Ok(EvaluatedToErr),
} }
} }
@ -1179,10 +1191,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Some(EvaluatedToOk) Some(EvaluatedToOk)
} else { } else {
debug!("evaluate_stack --> recursive, inductive"); debug!("evaluate_stack --> recursive, inductive");
match self.treat_inductive_cycle { Some(self.treat_inductive_cycle.into())
TreatInductiveCycleAs::Ambig => Some(EvaluatedToUnknown),
TreatInductiveCycleAs::Recur => Some(EvaluatedToRecur),
}
} }
} else { } else {
None None

View file

@ -11,7 +11,7 @@ pub(crate) struct Interval<T>(PhantomData<T>);
// `Interval<?1>: PartialOrd<Interval<?1>>` candidate which results // `Interval<?1>: PartialOrd<Interval<?1>>` candidate which results
// in a - currently inductive - cycle. // in a - currently inductive - cycle.
impl<T, Q> PartialEq<Q> for Interval<T> 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 //~| WARN this was previously accepted by the compiler but is being phased out
where where
T: Borrow<Q>, T: Borrow<Q>,

View file

@ -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 --> $DIR/warn-when-cycle-is-error-in-coherence.rs:13:1
| |
LL | #[derive(PartialEq, Default)] LL | #[derive(PartialEq, Default)]
@ -8,10 +8,11 @@ LL | impl<T, Q> PartialEq<Q> for Interval<T>
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the first impl is here | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the first impl is here
... ...
LL | Q: ?Sized + PartialOrd, 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! = 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: 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 note: the lint level is defined here
--> $DIR/warn-when-cycle-is-error-in-coherence.rs:1:9 --> $DIR/warn-when-cycle-is-error-in-coherence.rs:1:9
| |