Skip to content

Commit

Permalink
more nits
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Aug 15, 2023
1 parent ca49a37 commit 0e20155
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 49 deletions.
41 changes: 12 additions & 29 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
33 changes: 28 additions & 5 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand All @@ -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
Expand Down
33 changes: 21 additions & 12 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr
View file
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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
|
Expand Down

0 comments on commit 0e20155

Please sign in to comment.