Auto merge of rust-lang#136968 - oli-obk:bye-bye, r=<try>

Turn order dependent trait objects future incompat warning into a hard error

fixes rust-lang#56484

r? `@ghost`

will FCP when we have a crater result

Author: bors

compiler/rustc_lint_defs/src/builtin.rs

@@ -73,7 +73,6 @@ declare_lint_pass! {
         NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE,
         NON_CONTIGUOUS_RANGE_ENDPOINTS,
         NON_EXHAUSTIVE_OMITTED_PATTERNS,
-        ORDER_DEPENDENT_TRAIT_OBJECTS,
         OUT_OF_SCOPE_MACRO_CALLS,
         OVERLAPPING_RANGE_ENDPOINTS,
         PATTERNS_IN_FNS_WITHOUT_BODY,
@@ -1500,42 +1499,6 @@ declare_lint! {
     };
 }
 
-declare_lint! {
-    /// The `order_dependent_trait_objects` lint detects a trait coherency
-    /// violation that would allow creating two trait impls for the same
-    /// dynamic trait object involving marker traits.
-    ///
-    /// ### Example
-    ///
-    /// ```rust,compile_fail
-    /// pub trait Trait {}
-    ///
-    /// impl Trait for dyn Send + Sync { }
-    /// impl Trait for dyn Sync + Send { }
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// A previous bug caused the compiler to interpret traits with different
-    /// orders (such as `Send + Sync` and `Sync + Send`) as distinct types
-    /// when they were intended to be treated the same. This allowed code to
-    /// define separate trait implementations when there should be a coherence
-    /// error. This is a [future-incompatible] lint to transition this to a
-    /// hard error in the future. See [issue #56484] for more details.
-    ///
-    /// [issue #56484]: https://github.com/rust-lang/rust/issues/56484
-    /// [future-incompatible]: ../index.md#future-incompatible-lints
-    pub ORDER_DEPENDENT_TRAIT_OBJECTS,
-    Deny,
-    "trait-object types were treated as different depending on marker-trait order",
-    @future_incompatible = FutureIncompatibleInfo {
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
-        reference: "issue #56484 <https://github.com/rust-lang/rust/issues/56484>",
-    };
-}
-
 declare_lint! {
     /// The `coherence_leak_check` lint detects conflicting implementations of
     /// a trait that are only distinguished by the old leak-check code.

compiler/rustc_middle/src/query/mod.rs

@@ -985,12 +985,6 @@ rustc_queries! {
         separate_provide_extern
     }
 
-    query self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(
-        key: DefId
-    ) -> Option<ty::EarlyBinder<'tcx, ty::Ty<'tcx>>> {
-        desc { |tcx| "computing self type wrt issue #33140 `{}`", tcx.def_path_str(key) }
-    }
-
     /// Maps a `DefId` of a type to a list of its inherent impls.
     /// Contains implementations of methods that are inherent to a type.
     /// Methods in these implementations don't need to be exported.

compiler/rustc_middle/src/ty/mod.rs

@@ -1424,39 +1424,6 @@ pub enum ImplOverlapKind {
         /// Whether or not the impl is permitted due to the trait being a `#[marker]` trait
         marker: bool,
     },
-    /// These impls are allowed to overlap, but that raises an
-    /// issue #33140 future-compatibility warning (tracked in #56484).
-    ///
-    /// Some background: in Rust 1.0, the trait-object types `Send + Sync` (today's
-    /// `dyn Send + Sync`) and `Sync + Send` (now `dyn Sync + Send`) were different.
-    ///
-    /// The widely-used version 0.1.0 of the crate `traitobject` had accidentally relied on
-    /// that difference, doing what reduces to the following set of impls:
-    ///
-    /// ```compile_fail,(E0119)
-    /// trait Trait {}
-    /// impl Trait for dyn Send + Sync {}
-    /// impl Trait for dyn Sync + Send {}
-    /// ```
-    ///
-    /// Obviously, once we made these types be identical, that code causes a coherence
-    /// error and a fairly big headache for us. However, luckily for us, the trait
-    /// `Trait` used in this case is basically a marker trait, and therefore having
-    /// overlapping impls for it is sound.
-    ///
-    /// To handle this, we basically regard the trait as a marker trait, with an additional
-    /// future-compatibility warning. To avoid accidentally "stabilizing" this feature,
-    /// it has the following restrictions:
-    ///
-    /// 1. The trait must indeed be a marker-like trait (i.e., no items), and must be
-    /// positive impls.
-    /// 2. The trait-ref of both impls must be equal.
-    /// 3. The trait-ref of both impls must be a trait object type consisting only of
-    /// marker traits.
-    /// 4. Neither of the impls can have any where-clauses.
-    ///
-    /// Once `traitobject` 0.1.0 is no longer an active concern, this hack can be removed.
-    FutureCompatOrderDepTraitObjects,
 }
 
 /// Useful source information about where a desugared associated type for an
@@ -1630,7 +1597,7 @@ impl<'tcx> TyCtxt<'tcx> {
         })
     }
 
-    /// Returns `true` if the impls are the same polarity and the trait either
+    /// Returns `Some` if the impls are the same polarity and the trait either
     /// has no items or is annotated `#[marker]` and prevents item overrides.
     #[instrument(level = "debug", skip(self), ret)]
     pub fn impls_are_allowed_to_overlap(
@@ -1671,18 +1638,6 @@ impl<'tcx> TyCtxt<'tcx> {
             return Some(ImplOverlapKind::Permitted { marker: true });
         }
 
-        if let Some(self_ty1) =
-            self.self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(def_id1)
-            && let Some(self_ty2) =
-                self.self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(def_id2)
-        {
-            if self_ty1 == self_ty2 {
-                return Some(ImplOverlapKind::FutureCompatOrderDepTraitObjects);
-            } else {
-                debug!("found {self_ty1:?} != {self_ty2:?}");
-            }
-        }
-
         None
     }
 

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -1920,9 +1920,9 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
         let mut impl_candidate = None;
         for c in impls {
             if let Some(prev) = impl_candidate.replace(c) {
-                if self.prefer_lhs_over_victim(has_non_region_infer, c, prev) {
+                if self.prefer_lhs_over_victim(has_non_region_infer, c, prev.0) {
                     // Ok, prefer `c` over the previous entry
-                } else if self.prefer_lhs_over_victim(has_non_region_infer, prev, c) {
+                } else if self.prefer_lhs_over_victim(has_non_region_infer, prev, c.0) {
                     // Ok, keep `prev` instead of the new entry
                     impl_candidate = Some(prev);
                 } else {
@@ -1980,7 +1980,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
         &self,
         has_non_region_infer: bool,
         (lhs, lhs_evaluation): (DefId, EvaluationResult),
-        (victim, victim_evaluation): (DefId, EvaluationResult),
+        victim: DefId,
     ) -> bool {
         let tcx = self.tcx();
         // See if we can toss out `victim` based on specialization.
@@ -1996,14 +1996,6 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
         }
 
         match tcx.impls_are_allowed_to_overlap(lhs, victim) {
-            // For #33140 the impl headers must be exactly equal, the trait must not have
-            // any associated items and there are no where-clauses.
-            //
-            // We can just arbitrarily drop one of the impls.
-            Some(ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects) => {
-                assert_eq!(lhs_evaluation, victim_evaluation);
-                true
-            }
             // For candidates which already reference errors it doesn't really
             // matter what we do 🤷
             Some(ty::ImplOverlapKind::Permitted { marker: false }) => {

compiler/rustc_trait_selection/src/traits/specialize/mod.rs

@@ -20,7 +20,7 @@ use rustc_middle::bug;
 use rustc_middle::query::LocalCrate;
 use rustc_middle::ty::print::PrintTraitRefExt as _;
 use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt, TypingMode};
-use rustc_session::lint::builtin::{COHERENCE_LEAK_CHECK, ORDER_DEPENDENT_TRAIT_OBJECTS};
+use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, sym};
 use rustc_type_ir::solve::NoSolution;
 use specialization_graph::GraphExt;
@@ -557,13 +557,9 @@ fn report_conflicting_impls<'tcx>(
 
     let msg = || {
         format!(
-            "conflicting implementations of trait `{}`{}{}",
+            "conflicting implementations of trait `{}`{}",
             overlap.trait_ref.print_trait_sugared(),
             overlap.self_ty.map_or_else(String::new, |ty| format!(" for type `{ty}`")),
-            match used_to_be_allowed {
-                Some(FutureCompatOverlapErrorKind::OrderDepTraitObjects) => ": (E0119)",
-                _ => "",
-            }
         )
     };
 
@@ -588,7 +584,6 @@ fn report_conflicting_impls<'tcx>(
         }
         Some(kind) => {
             let lint = match kind {
-                FutureCompatOverlapErrorKind::OrderDepTraitObjects => ORDER_DEPENDENT_TRAIT_OBJECTS,
                 FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
             };
             tcx.node_span_lint(lint, tcx.local_def_id_to_hir_id(impl_def_id), impl_span, |err| {

compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs

@@ -12,7 +12,6 @@ use crate::traits;
 
 #[derive(Copy, Clone, Debug)]
 pub enum FutureCompatOverlapErrorKind {
-    OrderDepTraitObjects,
     LeakCheck,
 }
 
@@ -151,12 +150,6 @@ impl<'tcx> Children {
                 {
                     match overlap_kind {
                         ty::ImplOverlapKind::Permitted { marker: _ } => {}
-                        ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects => {
-                            *last_lint_mut = Some(FutureCompatOverlapError {
-                                error: create_overlap_error(overlap),
-                                kind: FutureCompatOverlapErrorKind::OrderDepTraitObjects,
-                            });
-                        }
                     }
 
                     return Ok((false, false));

compiler/rustc_ty_utils/src/ty.rs

@@ -6,13 +6,11 @@ use rustc_index::bit_set::DenseBitSet;
 use rustc_middle::bug;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::fold::fold_regions;
-use rustc_middle::ty::{
-    self, EarlyBinder, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, Upcast,
-};
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, Upcast};
 use rustc_span::DUMMY_SP;
 use rustc_span::def_id::{CRATE_DEF_ID, DefId, LocalDefId};
 use rustc_trait_selection::traits;
-use tracing::{debug, instrument};
+use tracing::instrument;
 
 #[instrument(level = "debug", skip(tcx), ret)]
 fn sized_constraint_for_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
@@ -257,57 +255,6 @@ fn param_env_normalized_for_post_analysis(tcx: TyCtxt<'_>, def_id: DefId) -> ty:
     typing_env.with_post_analysis_normalized(tcx).param_env
 }
 
-/// If the given trait impl enables exploiting the former order dependence of trait objects,
-/// returns its self type; otherwise, returns `None`.
-///
-/// See [`ty::ImplOverlapKind::FutureCompatOrderDepTraitObjects`] for more details.
-#[instrument(level = "debug", skip(tcx))]
-fn self_ty_of_trait_impl_enabling_order_dep_trait_object_hack(
-    tcx: TyCtxt<'_>,
-    def_id: DefId,
-) -> Option<EarlyBinder<'_, Ty<'_>>> {
-    let impl_ =
-        tcx.impl_trait_header(def_id).unwrap_or_else(|| bug!("called on inherent impl {def_id:?}"));
-
-    let trait_ref = impl_.trait_ref.skip_binder();
-    debug!(?trait_ref);
-
-    let is_marker_like = impl_.polarity == ty::ImplPolarity::Positive
-        && tcx.associated_item_def_ids(trait_ref.def_id).is_empty();
-
-    // Check whether these impls would be ok for a marker trait.
-    if !is_marker_like {
-        debug!("not marker-like!");
-        return None;
-    }
-
-    // impl must be `impl Trait for dyn Marker1 + Marker2 + ...`
-    if trait_ref.args.len() != 1 {
-        debug!("impl has args!");
-        return None;
-    }
-
-    let predicates = tcx.predicates_of(def_id);
-    if predicates.parent.is_some() || !predicates.predicates.is_empty() {
-        debug!(?predicates, "impl has predicates!");
-        return None;
-    }
-
-    let self_ty = trait_ref.self_ty();
-    let self_ty_matches = match self_ty.kind() {
-        ty::Dynamic(data, re, _) if re.is_static() => data.principal().is_none(),
-        _ => false,
-    };
-
-    if self_ty_matches {
-        debug!("MATCHES!");
-        Some(EarlyBinder::bind(self_ty))
-    } else {
-        debug!("non-matching self type");
-        None
-    }
-}
-
 /// Check if a function is async.
 fn asyncness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Asyncness {
     let node = tcx.hir_node_by_def_id(def_id);
@@ -367,7 +314,6 @@ pub(crate) fn provide(providers: &mut Providers) {
         adt_sized_constraint,
         param_env,
         param_env_normalized_for_post_analysis,
-        self_ty_of_trait_impl_enabling_order_dep_trait_object_hack,
         defaultness,
         unsizing_params_for_adt,
         ..*providers

tests/ui/lint/lint-incoherent-auto-trait-objects.rs

@@ -4,16 +4,13 @@ impl Foo for dyn Send {}
 
 impl Foo for dyn Send + Send {}
 //~^ ERROR conflicting implementations
-//~| hard error
 
 impl Foo for dyn Send + Sync {}
 
 impl Foo for dyn Sync + Send {}
 //~^ ERROR conflicting implementations
-//~| hard error
 
 impl Foo for dyn Send + Sync + Send {}
 //~^ ERROR conflicting implementations
-//~| hard error
 
 fn main() {}