Skip to content

Commit

Permalink
Make inductive cycles in coherence ambiguous always
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Jan 8, 2024
1 parent 0ee9cfd commit 8abf133
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 155 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Expand Up @@ -513,6 +513,11 @@ fn register_builtins(store: &mut LintStore) {
"converted into hard error, see PR #117984 \
<https://github.com/rust-lang/rust/pull/117984> for more information",
);
store.register_removed(
"coinductive_overlap_in_coherence",
"converted into hard error, see PR #118649 \
<https://github.com/rust-lang/rust/pull/118649> for more information",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
40 changes: 0 additions & 40 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Expand Up @@ -26,7 +26,6 @@ declare_lint_pass! {
BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
CENUM_IMPL_DROP_CAST,
COHERENCE_LEAK_CHECK,
COINDUCTIVE_OVERLAP_IN_COHERENCE,
CONFLICTING_REPR_HINTS,
CONST_EVALUATABLE_UNCHECKED,
CONST_ITEM_MUTATION,
Expand Down Expand Up @@ -4367,45 +4366,6 @@ declare_lint! {
@feature_gate = sym::type_privacy_lints;
}

declare_lint! {
/// The `coinductive_overlap_in_coherence` lint detects impls which are currently
/// considered not overlapping, but may be considered to overlap if support for
/// coinduction is added to the trait solver.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(coinductive_overlap_in_coherence)]
///
/// trait CyclicTrait {}
/// impl<T: CyclicTrait> CyclicTrait for T {}
///
/// trait Trait {}
/// impl<T: CyclicTrait> Trait for T {}
/// // conflicting impl with the above
/// impl Trait for u8 {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// 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,
Deny,
"impls that are not considered to overlap may be considered to \
overlap in the future",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #114040 <https://github.com/rust-lang/rust/issues/114040>",
};
}

declare_lint! {
/// The `unknown_or_malformed_diagnostic_attributes` lint detects unrecognized or otherwise malformed
/// diagnostic attributes.
Expand Down
64 changes: 6 additions & 58 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Expand Up @@ -10,7 +10,7 @@ use crate::solve::inspect::{InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor
use crate::solve::{deeply_normalize_for_diagnostics, inspect};
use crate::traits::engine::TraitEngineExt;
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::select::{IntercrateAmbiguityCause, TreatInductiveCycleAs};
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::structural_normalize::StructurallyNormalizeExt;
use crate::traits::NormalizeExt;
use crate::traits::SkipLeakCheck;
Expand All @@ -31,7 +31,6 @@ use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams};
use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor};
use rustc_session::lint::builtin::COINDUCTIVE_OVERLAP_IN_COHERENCE;
use rustc_span::symbol::sym;
use rustc_span::DUMMY_SP;
use std::fmt::Debug;
Expand Down Expand Up @@ -197,7 +196,7 @@ fn overlap<'tcx>(
.intercrate(true)
.with_next_trait_solver(tcx.next_trait_solver_in_coherence())
.build();
let selcx = &mut SelectionContext::new(&infcx);
let selcx = &mut SelectionContext::with_treat_inductive_cycle_as_ambig(&infcx);
if track_ambiguity_causes.is_yes() {
selcx.enable_tracking_intercrate_ambiguity_causes();
}
Expand All @@ -224,61 +223,10 @@ fn overlap<'tcx>(
);

if overlap_mode.use_implicit_negative() {
for mode in [TreatInductiveCycleAs::Ambig, TreatInductiveCycleAs::Recur] {
if let Some(failing_obligation) = selcx.with_treat_inductive_cycle_as(mode, |selcx| {
impl_intersection_has_impossible_obligation(selcx, &obligations)
}) {
if matches!(mode, TreatInductiveCycleAs::Recur) {
let first_local_impl = impl1_header
.impl_def_id
.as_local()
.or(impl2_header.impl_def_id.as_local())
.expect("expected one of the impls to be local");
infcx.tcx.struct_span_lint_hir(
COINDUCTIVE_OVERLAP_IN_COHERENCE,
infcx.tcx.local_def_id_to_hir_id(first_local_impl),
infcx.tcx.def_span(first_local_impl),
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_trait_sugared(),
trait_ref.self_ty()
)
}
None => format!(
"for `{}`",
infcx.resolve_vars_if_possible(impl1_header.self_ty)
),
},
),
|lint| {
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",
)
.span_label(
infcx.tcx.def_span(impl2_header.impl_def_id),
"the second impl is here",
);
lint.note(format!(
"`{}` may be considered to hold in future releases, \
causing the impls to overlap",
infcx.resolve_vars_if_possible(failing_obligation.predicate)
));
},
);
}

return None;
}
if let Some(_failing_obligation) =
impl_intersection_has_impossible_obligation(selcx, &obligations)
{
return None;
}
}

Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Expand Up @@ -239,20 +239,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

// Sets the `TreatInductiveCycleAs` mode temporarily in the selection context
pub fn with_treat_inductive_cycle_as<T>(
&mut self,
treat_inductive_cycle: TreatInductiveCycleAs,
f: impl FnOnce(&mut Self) -> T,
) -> T {
pub fn with_treat_inductive_cycle_as_ambig(
infcx: &'cx InferCtxt<'tcx>,
) -> SelectionContext<'cx, 'tcx> {
// 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);
self.treat_inductive_cycle = treat_inductive_cycle;
value
assert!(infcx.intercrate);
SelectionContext {
treat_inductive_cycle: TreatInductiveCycleAs::Ambig,
..SelectionContext::new(infcx)
}
}

pub fn with_query_mode(
Expand Down
7 changes: 2 additions & 5 deletions tests/ui/coherence/warn-when-cycle-is-error-in-coherence.rs
View file
@@ -1,18 +1,15 @@
#![deny(coinductive_overlap_in_coherence)]

use std::borrow::Borrow;
use std::cmp::Ordering;
use std::marker::PhantomData;

#[derive(PartialEq, Default)]
//~^ ERROR conflicting implementations of trait `PartialEq<Interval<_>>` for type `Interval<_>`
pub(crate) struct Interval<T>(PhantomData<T>);

// This impl overlaps with the `derive` unless we reject the nested
// `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>
//~^ 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>,
Q: ?Sized + PartialOrd,
Expand Down
46 changes: 6 additions & 40 deletions tests/ui/coherence/warn-when-cycle-is-error-in-coherence.stderr
View file
@@ -1,51 +1,17 @@
error: implementations of `PartialEq<Interval<_>>` for `Interval<_>` will conflict in the future
--> $DIR/warn-when-cycle-is-error-in-coherence.rs:13:1
error[E0119]: conflicting implementations of trait `PartialEq<Interval<_>>` for type `Interval<_>`
--> $DIR/warn-when-cycle-is-error-in-coherence.rs:5:10
|
LL | #[derive(PartialEq, Default)]
| --------- the second impl is here
| ^^^^^^^^^ conflicting implementation for `Interval<_>`
...
LL | / impl<T, Q> PartialEq<Q> for Interval<T>
LL | |
LL | |
LL | | where
LL | | T: Borrow<Q>,
LL | | Q: ?Sized + PartialOrd,
| |___________________________^ the first impl is here
| |___________________________- first implementation here
|
= 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: `Interval<_>: PartialOrd` may be considered to hold in future releases, causing the impls to overlap
note: the lint level is defined here
--> $DIR/warn-when-cycle-is-error-in-coherence.rs:1:9
|
LL | #![deny(coinductive_overlap_in_coherence)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

Future incompatibility report: Future breakage diagnostic:
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)]
| --------- the second impl is here
...
LL | / impl<T, Q> PartialEq<Q> for Interval<T>
LL | |
LL | |
LL | | where
LL | | T: Borrow<Q>,
LL | | Q: ?Sized + PartialOrd,
| |___________________________^ the first impl is here
|
= 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: `Interval<_>: PartialOrd` may be considered to hold in future releases, causing the impls to overlap
note: the lint level is defined here
--> $DIR/warn-when-cycle-is-error-in-coherence.rs:1:9
|
LL | #![deny(coinductive_overlap_in_coherence)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0119`.

0 comments on commit 8abf133

Please sign in to comment.