Auto merge of #106253 - nbdd0121:upcast, r=compiler-errors

Skip possible where_clause_object_safety lints when checking `multiple_supertrait_upcastable`

Fix #106247

To achieve this, I lifted the `WhereClauseReferencesSelf` out from `object_safety_violations` and move it into `is_object_safe` (which is changed to a new query).

cc `@dtolnay`
r? `@compiler-errors`

Author: bors

compiler/rustc_error_messages/locales/en-US/lint.ftl

@@ -100,6 +100,8 @@ lint_cstring_ptr = getting the inner pointer of a temporary `CString`
     .note = pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
     .help = for more information, see https://doc.rust-lang.org/reference/destructors.html
 
+lint_multple_supertrait_upcastable = `{$ident}` is object-safe and has multiple supertraits
+
 lint_identifier_non_ascii_char = identifier contains non-ASCII characters
 
 lint_identifier_uncommon_codepoints = identifier contains uncommon Unicode codepoints

compiler/rustc_feature/src/active.rs

@@ -160,6 +160,8 @@ declare_features! (
     (active, intrinsics, "1.0.0", None, None),
     /// Allows using `#[lang = ".."]` attribute for linking items to special compiler logic.
     (active, lang_items, "1.0.0", None, None),
+    /// Allows the `multiple_supertrait_upcastable` lint.
+    (active, multiple_supertrait_upcastable, "CURRENT_RUSTC_VERSION", None, None),
     /// Allows using `#[omit_gdb_pretty_printer_section]`.
     (active, omit_gdb_pretty_printer_section, "1.5.0", None, None),
     /// Allows using `#[prelude_import]` on glob `use` items.

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -841,7 +841,7 @@ fn check_object_unsafe_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitItem
         _ => {}
     }
     if !trait_should_be_self.is_empty() {
-        if tcx.object_safety_violations(trait_def_id).is_empty() {
+        if tcx.check_is_object_safe(trait_def_id) {
             return;
         }
         let sugg = trait_should_be_self.iter().map(|span| (*span, "Self".to_string())).collect();

compiler/rustc_hir_analysis/src/coherence/mod.rs

@@ -169,7 +169,7 @@ fn check_object_overlap<'tcx>(
         });
 
         for component_def_id in component_def_ids {
-            if !tcx.is_object_safe(component_def_id) {
+            if !tcx.check_is_object_safe(component_def_id) {
                 // Without the 'object_safe_for_dispatch' feature this is an error
                 // which will be reported by wfcheck. Ignore it here.
                 // This is tested by `coherence-impl-trait-for-trait-object-safe.rs`.

compiler/rustc_hir_typeck/src/coercion.rs

@@ -1823,7 +1823,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                         .trait_ref()
                         .and_then(|t| t.trait_def_id())
                         .map_or(false, |def_id| {
-                            fcx.tcx.object_safety_violations(def_id).is_empty()
+                            fcx.tcx.check_is_object_safe(def_id)
                         })
                 })
             }

compiler/rustc_lint/src/lib.rs

@@ -64,6 +64,7 @@ mod let_underscore;
 mod levels;
 mod lints;
 mod methods;
+mod multiple_supertrait_upcastable;
 mod non_ascii_idents;
 mod non_fmt_panic;
 mod nonstandard_style;
@@ -98,6 +99,7 @@ use hidden_unicode_codepoints::*;
 use internal::*;
 use let_underscore::*;
 use methods::*;
+use multiple_supertrait_upcastable::*;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
 use nonstandard_style::*;
@@ -232,6 +234,7 @@ late_lint_methods!(
             InvalidAtomicOrdering: InvalidAtomicOrdering,
             NamedAsmLabels: NamedAsmLabels,
             OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
+            MultipleSupertraitUpcastable: MultipleSupertraitUpcastable,
         ]
     ]
 );

compiler/rustc_lint/src/lints.rs

@@ -917,6 +917,13 @@ pub struct CStringPtr {
     pub unwrap: Span,
 }
 
+// multiple_supertrait_upcastable.rs
+#[derive(LintDiagnostic)]
+#[diag(lint_multple_supertrait_upcastable)]
+pub struct MultipleSupertraitUpcastable {
+    pub ident: Ident,
+}
+
 // non_ascii_idents.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_identifier_non_ascii_char)]

compiler/rustc_lint/src/multiple_supertrait_upcastable.rs

@@ -0,0 +1,60 @@
+use crate::{LateContext, LateLintPass, LintContext};
+
+use rustc_hir as hir;
+use rustc_span::sym;
+
+declare_lint! {
+    /// The `multiple_supertrait_upcastable` lint detects when an object-safe trait has multiple
+    /// supertraits.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// trait A {}
+    /// trait B {}
+    ///
+    /// #[warn(multiple_supertrait_upcastable)]
+    /// trait C: A + B {}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// To support upcasting with multiple supertraits, we need to store multiple vtables and this
+    /// can result in extra space overhead, even if no code actually uses upcasting.
+    /// This lint allows users to identify when such scenarios occur and to decide whether the
+    /// additional overhead is justified.
+    pub MULTIPLE_SUPERTRAIT_UPCASTABLE,
+    Allow,
+    "detect when an object-safe trait has multiple supertraits",
+    @feature_gate = sym::multiple_supertrait_upcastable;
+}
+
+declare_lint_pass!(MultipleSupertraitUpcastable => [MULTIPLE_SUPERTRAIT_UPCASTABLE]);
+
+impl<'tcx> LateLintPass<'tcx> for MultipleSupertraitUpcastable {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
+        let def_id = item.owner_id.to_def_id();
+        // NOTE(nbdd0121): use `object_safety_violations` instead of `check_is_object_safe` because
+        // the latter will report `where_clause_object_safety` lint.
+        if let hir::ItemKind::Trait(_, _, _, _, _) = item.kind
+            && cx.tcx.object_safety_violations(def_id).is_empty()
+        {
+            let direct_super_traits_iter = cx.tcx
+                    .super_predicates_of(def_id)
+                    .predicates
+                    .into_iter()
+                    .filter_map(|(pred, _)| pred.to_opt_poly_trait_pred());
+            if direct_super_traits_iter.count() > 1 {
+                cx.emit_spanned_lint(
+                    MULTIPLE_SUPERTRAIT_UPCASTABLE,
+                    cx.tcx.def_span(def_id),
+                    crate::lints::MultipleSupertraitUpcastable {
+                        ident: item.ident
+                    },
+                );
+            }
+        }
+    }
+}

compiler/rustc_middle/src/query/mod.rs

@@ -1274,6 +1274,9 @@ rustc_queries! {
     query object_safety_violations(trait_id: DefId) -> &'tcx [traits::ObjectSafetyViolation] {
         desc { |tcx| "determining object safety of trait `{}`", tcx.def_path_str(trait_id) }
     }
+    query check_is_object_safe(trait_id: DefId) -> bool {
+        desc { |tcx| "checking if trait `{}` is object safe", tcx.def_path_str(trait_id) }
+    }
 
     /// Gets the ParameterEnvironment for a given item; this environment
     /// will be in "user-facing" mode, meaning that it is suitable for

compiler/rustc_middle/src/ty/mod.rs

@@ -2458,10 +2458,6 @@ impl<'tcx> TyCtxt<'tcx> {
             }
     }
 
-    pub fn is_object_safe(self, key: DefId) -> bool {
-        self.object_safety_violations(key).is_empty()
-    }
-
     #[inline]
     pub fn is_const_fn_raw(self, def_id: DefId) -> bool {
         matches!(

compiler/rustc_span/src/symbol.rs

@@ -955,6 +955,7 @@ symbols! {
         mul,
         mul_assign,
         mul_with_overflow,
+        multiple_supertrait_upcastable,
         must_not_suspend,
         must_use,
         naked,

compiler/rustc_trait_selection/src/solve/mod.rs

@@ -361,7 +361,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
     }
 
     fn compute_object_safe_goal(&mut self, trait_def_id: DefId) -> QueryResult<'tcx> {
-        if self.tcx().is_object_safe(trait_def_id) {
+        if self.tcx().check_is_object_safe(trait_def_id) {
             self.make_canonical_response(Certainty::Yes)
         } else {
             Err(NoSolution)

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -1748,7 +1748,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 // If the `dyn Trait` is not object safe, do not suggest `Box<dyn Trait>`.
                 predicates
                     .principal_def_id()
-                    .map_or(true, |def_id| self.tcx.object_safety_violations(def_id).is_empty())
+                    .map_or(true, |def_id| self.tcx.check_is_object_safe(def_id))
             }
             // We only want to suggest `impl Trait` to `dyn Trait`s.
             // For example, `fn foo() -> str` needs to be filtered out.

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -369,7 +369,7 @@ impl<'a, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'tcx> {
                 }
 
                 ty::PredicateKind::ObjectSafe(trait_def_id) => {
-                    if !self.selcx.tcx().is_object_safe(trait_def_id) {
+                    if !self.selcx.tcx().check_is_object_safe(trait_def_id) {
                         ProcessResult::Error(CodeSelectionError(Unimplemented))
                     } else {
                         ProcessResult::Changed(vec![])

compiler/rustc_trait_selection/src/traits/object_safety.rs

@@ -62,6 +62,37 @@ fn object_safety_violations(tcx: TyCtxt<'_>, trait_def_id: DefId) -> &'_ [Object
     )
 }
 
+fn check_is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool {
+    let violations = tcx.object_safety_violations(trait_def_id);
+
+    if violations.is_empty() {
+        return true;
+    }
+
+    // If the trait contains any other violations, then let the error reporting path
+    // report it instead of emitting a warning here.
+    if violations.iter().all(|violation| {
+        matches!(
+            violation,
+            ObjectSafetyViolation::Method(_, MethodViolationCode::WhereClauseReferencesSelf, _)
+        )
+    }) {
+        for violation in violations {
+            if let ObjectSafetyViolation::Method(
+                _,
+                MethodViolationCode::WhereClauseReferencesSelf,
+                span,
+            ) = violation
+            {
+                lint_object_unsafe_trait(tcx, *span, trait_def_id, &violation);
+            }
+        }
+        return true;
+    }
+
+    false
+}
+
 /// We say a method is *vtable safe* if it can be invoked on a trait
 /// object. Note that object-safe traits can have some
 /// non-vtable-safe methods, so long as they require `Self: Sized` or
@@ -93,19 +124,6 @@ fn object_safety_violations_for_trait(
             object_safety_violation_for_method(tcx, trait_def_id, &item)
                 .map(|(code, span)| ObjectSafetyViolation::Method(item.name, code, span))
         })
-        .filter(|violation| {
-            if let ObjectSafetyViolation::Method(
-                _,
-                MethodViolationCode::WhereClauseReferencesSelf,
-                span,
-            ) = violation
-            {
-                lint_object_unsafe_trait(tcx, *span, trait_def_id, &violation);
-                false
-            } else {
-                true
-            }
-        })
         .collect();
 
     // Check the trait itself.
@@ -866,5 +884,6 @@ pub fn contains_illegal_impl_trait_in_trait<'tcx>(
 }
 
 pub fn provide(providers: &mut ty::query::Providers) {
-    *providers = ty::query::Providers { object_safety_violations, ..*providers };
+    *providers =
+        ty::query::Providers { object_safety_violations, check_is_object_safe, ..*providers };
 }

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

@@ -466,7 +466,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     if let Some(principal) = data.principal() {
                         if !self.infcx.tcx.features().object_safe_for_dispatch {
                             principal.with_self_ty(self.tcx(), self_ty)
-                        } else if self.tcx().is_object_safe(principal.def_id()) {
+                        } else if self.tcx().check_is_object_safe(principal.def_id()) {
                             principal.with_self_ty(self.tcx(), self_ty)
                         } else {
                             return;

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

@@ -1009,7 +1009,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             // `T` -> `Trait`
             (_, &ty::Dynamic(ref data, r, ty::Dyn)) => {
                 let mut object_dids = data.auto_traits().chain(data.principal_def_id());
-                if let Some(did) = object_dids.find(|did| !tcx.is_object_safe(*did)) {
+                if let Some(did) = object_dids.find(|did| !tcx.check_is_object_safe(*did)) {
                     return Err(TraitNotObjectSafe(did));
                 }
 

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

@@ -797,7 +797,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 }
 
                 ty::PredicateKind::ObjectSafe(trait_def_id) => {
-                    if self.tcx().is_object_safe(trait_def_id) {
+                    if self.tcx().check_is_object_safe(trait_def_id) {
                         Ok(EvaluatedToOk)
                     } else {
                         Ok(EvaluatedToErr)

compiler/rustc_traits/src/chalk/db.rs

@@ -580,7 +580,7 @@ impl<'tcx> chalk_solve::RustIrDatabase<RustInterner<'tcx>> for RustIrDatabase<'t
     }
 
     fn is_object_safe(&self, trait_id: chalk_ir::TraitId<RustInterner<'tcx>>) -> bool {
-        self.interner.tcx.is_object_safe(trait_id.0)
+        self.interner.tcx.check_is_object_safe(trait_id.0)
     }
 
     fn hidden_opaque_type(

library/alloc/src/lib.rs

@@ -87,6 +87,7 @@
 #![warn(missing_debug_implementations)]
 #![warn(missing_docs)]
 #![allow(explicit_outlives_requirements)]
+#![cfg_attr(not(bootstrap), warn(multiple_supertrait_upcastable))]
 //
 // Library features:
 #![feature(alloc_layout_extra)]
@@ -195,6 +196,7 @@
 #![feature(c_unwind)]
 #![feature(with_negative_coherence)]
 #![cfg_attr(test, feature(panic_update_hook))]
+#![cfg_attr(not(bootstrap), feature(multiple_supertrait_upcastable))]
 //
 // Rustdoc features:
 #![feature(doc_cfg)]

library/core/src/error.rs

@@ -28,6 +28,7 @@ use crate::fmt::{Debug, Display};
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "Error")]
 #[rustc_has_incoherent_inherent_impls]
+#[cfg_attr(not(bootstrap), allow(multiple_supertrait_upcastable))]
 pub trait Error: Debug + Display {
     /// The lower-level source of this error, if any.
     ///

library/core/src/lib.rs

@@ -95,6 +95,7 @@
 #![warn(missing_docs)]
 #![allow(explicit_outlives_requirements)]
 #![allow(incomplete_features)]
+#![cfg_attr(not(bootstrap), warn(multiple_supertrait_upcastable))]
 //
 // Library features:
 #![feature(const_align_offset)]
@@ -235,6 +236,7 @@
 #![feature(unsized_fn_params)]
 #![feature(asm_const)]
 #![feature(const_transmute_copy)]
+#![cfg_attr(not(bootstrap), feature(multiple_supertrait_upcastable))]
 //
 // Target features:
 #![feature(arm_target_feature)]

tests/ui/feature-gates/feature-gate-multiple_supertrait_upcastable.rs

@@ -0,0 +1,12 @@
+// check-pass
+
+#![deny(multiple_supertrait_upcastable)]
+//~^ WARNING unknown lint: `multiple_supertrait_upcastable`
+//~| WARNING unknown lint: `multiple_supertrait_upcastable`
+//~| WARNING unknown lint: `multiple_supertrait_upcastable`
+#![warn(multiple_supertrait_upcastable)]
+//~^ WARNING unknown lint: `multiple_supertrait_upcastable`
+//~| WARNING unknown lint: `multiple_supertrait_upcastable`
+//~| WARNING unknown lint: `multiple_supertrait_upcastable`
+
+fn main() {}