Auto merge of rust-lang#112320 - compiler-errors:do-not-impl-via-obj, r=lcnr

Add `implement_via_object` to `rustc_deny_explicit_impl` to control object candidate assembly

Some built-in traits are special, since they are used to prove facts about the program that are important for later phases of compilation such as codegen and CTFE. For example, the `Unsize` trait is used to assert to the compiler that we are able to unsize a type into another type. It doesn't have any methods because it doesn't actually *instruct* the compiler how to do this unsizing, but this is later used (alongside an exhaustive match of combinations of unsizeable types) during codegen to generate unsize coercion code.

Due to this, these built-in traits are incompatible with the type erasure provided by object types. For example, the existence of `dyn Unsize<T>` does not mean that the compiler is able to unsize `Box<dyn Unsize<T>>` into `Box<T>`, since `Unsize` is a *witness* to the fact that a type can be unsized, and it doesn't actually encode that unsizing operation in its vtable as mentioned above.

The old trait solver gets around this fact by having complex control flow that never considers object bounds for certain built-in traits:
https://github.com/rust-lang/rust/blob/2f896da247e0ee8f0bef7cd7c54cfbea255b9f68/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs#L61-L132

However, candidate assembly in the new solver is much more lovely, and I'd hate to add this list of opt-out cases into the new solver. Instead of maintaining this complex and hard-coded control flow, instead we can make this a property of the trait via a built-in attribute. We already have such a build attribute that's applied to every single trait that we care about: `rustc_deny_explicit_impl`. This PR adds `implement_via_object` as a meta-item to that attribute that allows us to opt a trait out of object-bound candidate assembly as well.

r? `@lcnr`

Author: bors

compiler/rustc_feature/src/builtin_attrs.rs

@@ -705,7 +705,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         "#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl."
     ),
     rustc_attr!(
-        rustc_deny_explicit_impl, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: false,
+        rustc_deny_explicit_impl,
+        AttributeType::Normal,
+        template!(List: "implement_via_object = (true|false)"),
+        ErrorFollowing,
+        @only_local: true,
         "#[rustc_deny_explicit_impl] enforces that a trait can have no user-provided impls"
     ),
     rustc_attr!(

compiler/rustc_hir_analysis/src/coherence/mod.rs

@@ -10,7 +10,6 @@ use rustc_errors::{error_code, struct_span_err};
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
-use rustc_span::sym;
 use rustc_trait_selection::traits;
 
 mod builtin;
@@ -44,7 +43,7 @@ fn enforce_trait_manually_implementable(
     let impl_header_span = tcx.def_span(impl_def_id);
 
     // Disallow *all* explicit impls of traits marked `#[rustc_deny_explicit_impl]`
-    if tcx.has_attr(trait_def_id, sym::rustc_deny_explicit_impl) {
+    if tcx.trait_def(trait_def_id).deny_explicit_impl {
         let trait_name = tcx.item_name(trait_def_id);
         let mut err = struct_span_err!(
             tcx.sess,

compiler/rustc_hir_analysis/src/collect.rs

@@ -991,6 +991,50 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
             no_dups.then_some(list)
         });
 
+    let mut deny_explicit_impl = false;
+    let mut implement_via_object = true;
+    if let Some(attr) = tcx.get_attr(def_id, sym::rustc_deny_explicit_impl) {
+        deny_explicit_impl = true;
+        let mut seen_attr = false;
+        for meta in attr.meta_item_list().iter().flatten() {
+            if let Some(meta) = meta.meta_item()
+                && meta.name_or_empty() == sym::implement_via_object
+                && let Some(lit) = meta.name_value_literal()
+            {
+                if seen_attr {
+                    tcx.sess.span_err(
+                        meta.span,
+                        "duplicated `implement_via_object` meta item",
+                    );
+                }
+                seen_attr = true;
+
+                match lit.symbol {
+                    kw::True => {
+                        implement_via_object = true;
+                    }
+                    kw::False => {
+                        implement_via_object = false;
+                    }
+                    _ => {
+                        tcx.sess.span_err(
+                            meta.span,
+                            format!("unknown literal passed to `implement_via_object` attribute: {}", lit.symbol),
+                        );
+                    }
+                }
+            } else {
+                tcx.sess.span_err(
+                    meta.span(),
+                    format!("unknown meta item passed to `rustc_deny_explicit_impl` {:?}", meta),
+                );
+            }
+        }
+        if !seen_attr {
+            tcx.sess.span_err(attr.span, "missing `implement_via_object` meta item");
+        }
+    }
+
     ty::TraitDef {
         def_id: def_id.to_def_id(),
         unsafety,
@@ -1001,6 +1045,8 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
         skip_array_during_method_dispatch,
         specialization_kind,
         must_implement_one_of,
+        implement_via_object,
+        deny_explicit_impl,
     }
 }
 

compiler/rustc_middle/src/ty/trait_def.rs

@@ -52,6 +52,16 @@ pub struct TraitDef {
     /// List of functions from `#[rustc_must_implement_one_of]` attribute one of which
     /// must be implemented.
     pub must_implement_one_of: Option<Box<[Ident]>>,
+
+    /// Whether to add a builtin `dyn Trait: Trait` implementation.
+    /// This is enabled for all traits except ones marked with
+    /// `#[rustc_deny_explicit_impl(implement_via_object = false)]`.
+    pub implement_via_object: bool,
+
+    /// Whether a trait is fully built-in, and any implementation is disallowed.
+    /// This only applies to built-in traits, and is marked via
+    /// `#[rustc_deny_explicit_impl(implement_via_object = ...)]`.
+    pub deny_explicit_impl: bool,
 }
 
 /// Whether this trait is treated specially by the standard library

compiler/rustc_passes/messages.ftl

@@ -102,9 +102,6 @@ passes_const_impl_const_trait =
     const `impl`s must be for traits marked with `#[const_trait]`
     .note = this trait must be annotated with `#[const_trait]`
 
-passes_const_trait =
-    attribute should be applied to a trait
-
 passes_continue_labeled_block =
     `continue` pointing to a labeled block
     .label = labeled blocks cannot be `continue`'d

compiler/rustc_passes/src/check_attr.rs

@@ -110,9 +110,6 @@ impl CheckAttrVisitor<'_> {
                 sym::no_coverage => self.check_no_coverage(hir_id, attr, span, target),
                 sym::non_exhaustive => self.check_non_exhaustive(hir_id, attr, span, target),
                 sym::marker => self.check_marker(hir_id, attr, span, target),
-                sym::rustc_must_implement_one_of => {
-                    self.check_rustc_must_implement_one_of(attr, span, target)
-                }
                 sym::target_feature => self.check_target_feature(hir_id, attr, span, target),
                 sym::thread_local => self.check_thread_local(attr, span, target),
                 sym::track_caller => {
@@ -159,12 +156,14 @@ impl CheckAttrVisitor<'_> {
                 | sym::rustc_dirty
                 | sym::rustc_if_this_changed
                 | sym::rustc_then_this_would_need => self.check_rustc_dirty_clean(&attr),
-                sym::rustc_coinductive => self.check_rustc_coinductive(&attr, span, target),
+                sym::rustc_coinductive
+                | sym::rustc_must_implement_one_of
+                | sym::rustc_deny_explicit_impl
+                | sym::const_trait => self.check_must_be_applied_to_trait(&attr, span, target),
                 sym::cmse_nonsecure_entry => {
                     self.check_cmse_nonsecure_entry(hir_id, attr, span, target)
                 }
                 sym::collapse_debuginfo => self.check_collapse_debuginfo(attr, span, target),
-                sym::const_trait => self.check_const_trait(attr, span, target),
                 sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
                 sym::must_use => self.check_must_use(hir_id, &attr, target),
                 sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target),
@@ -567,25 +566,6 @@ impl CheckAttrVisitor<'_> {
         }
     }
 
-    /// Checks if the `#[rustc_must_implement_one_of]` attribute on a `target` is valid. Returns `true` if valid.
-    fn check_rustc_must_implement_one_of(
-        &self,
-        attr: &Attribute,
-        span: Span,
-        target: Target,
-    ) -> bool {
-        match target {
-            Target::Trait => true,
-            _ => {
-                self.tcx.sess.emit_err(errors::AttrShouldBeAppliedToTrait {
-                    attr_span: attr.span,
-                    defn_span: span,
-                });
-                false
-            }
-        }
-    }
-
     /// Checks if the `#[target_feature]` attribute on `item` is valid. Returns `true` if valid.
     fn check_target_feature(
         &self,
@@ -1591,8 +1571,8 @@ impl CheckAttrVisitor<'_> {
         }
     }
 
-    /// Checks if the `#[rustc_coinductive]` attribute is applied to a trait.
-    fn check_rustc_coinductive(&self, attr: &Attribute, span: Span, target: Target) -> bool {
+    /// Checks if the attribute is applied to a trait.
+    fn check_must_be_applied_to_trait(&self, attr: &Attribute, span: Span, target: Target) -> bool {
         match target {
             Target::Trait => true,
             _ => {
@@ -1986,17 +1966,6 @@ impl CheckAttrVisitor<'_> {
         }
     }
 
-    /// `#[const_trait]` only applies to traits.
-    fn check_const_trait(&self, attr: &Attribute, _span: Span, target: Target) -> bool {
-        match target {
-            Target::Trait => true,
-            _ => {
-                self.tcx.sess.emit_err(errors::ConstTrait { attr_span: attr.span });
-                false
-            }
-        }
-    }
-
     fn check_stability_promotable(&self, attr: &Attribute, _span: Span, target: Target) -> bool {
         match target {
             Target::Expression => {

compiler/rustc_passes/src/errors.rs

@@ -610,13 +610,6 @@ pub struct RustcStdInternalSymbol {
     pub span: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag(passes_const_trait)]
-pub struct ConstTrait {
-    #[primary_span]
-    pub attr_span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag(passes_link_ordinal)]
 pub struct LinkOrdinal {

compiler/rustc_span/src/symbol.rs

@@ -816,6 +816,7 @@ symbols! {
         impl_trait_in_bindings,
         impl_trait_in_fn_trait_return,
         impl_trait_projections,
+        implement_via_object,
         implied_by,
         import,
         import_name_type,

compiler/rustc_trait_selection/src/solve/assembly/mod.rs

@@ -640,6 +640,11 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         goal: Goal<'tcx, G>,
         candidates: &mut Vec<Candidate<'tcx>>,
     ) {
+        let tcx = self.tcx();
+        if !tcx.trait_def(goal.predicate.trait_def_id(tcx)).implement_via_object {
+            return;
+        }
+
         let self_ty = goal.predicate.self_ty();
         let bounds = match *self_ty.kind() {
             ty::Bool
@@ -672,7 +677,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
             ty::Dynamic(bounds, ..) => bounds,
         };
 
-        let tcx = self.tcx();
         let own_bounds: FxIndexSet<_> =
             bounds.iter().map(|bound| bound.with_self_ty(tcx, self_ty)).collect();
         for assumption in elaborate(tcx, own_bounds.iter().copied())

compiler/rustc_trait_selection/src/traits/project.rs

@@ -1607,6 +1607,10 @@ fn assemble_candidates_from_object_ty<'cx, 'tcx>(
 
     let tcx = selcx.tcx();
 
+    if !tcx.trait_def(obligation.predicate.trait_def_id(tcx)).implement_via_object {
+        return;
+    }
+
     let self_ty = obligation.predicate.self_ty();
     let object_ty = selcx.infcx.shallow_resolve(self_ty);
     let data = match object_ty.kind() {