From 8abf133c4b158a0c1b720c28cb25c482490c104f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 5 Dec 2023 17:29:36 +0000 Subject: [PATCH] Make inductive cycles in coherence ambiguous always --- compiler/rustc_lint/src/lib.rs | 5 ++ compiler/rustc_lint_defs/src/builtin.rs | 40 ------------ .../src/traits/coherence.rs | 64 ++----------------- .../src/traits/select/mod.rs | 20 +++--- .../warn-when-cycle-is-error-in-coherence.rs | 7 +- ...rn-when-cycle-is-error-in-coherence.stderr | 46 ++----------- 6 files changed, 27 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 76c630fc456a2..a9996e4a155bf 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -513,6 +513,11 @@ fn register_builtins(store: &mut LintStore) { "converted into hard error, see PR #117984 \ for more information", ); + store.register_removed( + "coinductive_overlap_in_coherence", + "converted into hard error, see PR #118649 \ + for more information", + ); } fn register_internals(store: &mut LintStore) { diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 19da51b7dcfa3..c74d4bb651c04 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -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, @@ -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 CyclicTrait for T {} - /// - /// trait Trait {} - /// impl 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 ", - }; -} - declare_lint! { /// The `unknown_or_malformed_diagnostic_attributes` lint detects unrecognized or otherwise malformed /// diagnostic attributes. diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 533fe32f70d87..ecbb92ca5b996 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -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; @@ -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; @@ -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(); } @@ -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; } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index c45925295ee72..61fe2c8efe364 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -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( - &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( 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 01f7d6ce90101..c50bbcec52157 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 @@ -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>` for type `Interval<_>` pub(crate) struct Interval(PhantomData); // This impl overlaps with the `derive` unless we reject the nested // `Interval: PartialOrd>` candidate which results -// in a - currently inductive - cycle. +// in a -- currently inductive -- cycle. impl PartialEq for Interval -//~^ 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, Q: ?Sized + PartialOrd, 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 4535b6f681152..af4dbfcad0e20 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,51 +1,17 @@ -error: implementations of `PartialEq>` 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>` 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 PartialEq for Interval -LL | | -LL | | LL | | where LL | | T: Borrow, 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 - = 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>` 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 PartialEq for Interval -LL | | -LL | | -LL | | where -LL | | T: Borrow, -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 - = 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`.