Merge attrs, better validation

rust-lang · Jun 20, 2023 · ca68cf0 · ca68cf0
1 parent 91e5c3f
commit ca68cf0
Show file tree

Hide file tree

Showing 16 changed files with 140 additions and 73 deletions.
diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs
@@ -705,14 +705,13 @@ 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: true,
+        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!(
-        rustc_do_not_implement_via_object, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: true,
-        "#[rustc_do_not_implement_via_object] marks a trait so that `dyn Trait` does not \
-        implement `Trait` (without requiring `Sized` as a supertrait)"
-    ),
     rustc_attr!(
         rustc_has_incoherent_inherent_impls, AttributeType::Normal, template!(Word), ErrorFollowing,
         "#[rustc_has_incoherent_inherent_impls] allows the addition of incoherent inherent impls for \

diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs
@@ -985,8 +985,50 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
 
             no_dups.then_some(list)
         });
-    let do_not_implement_via_object = tcx.has_attr(def_id, sym::rustc_do_not_implement_via_object);
-    let deny_explicit_impl = tcx.has_attr(def_id, sym::rustc_deny_explicit_impl);
+
+    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(),

diff --git a/compiler/rustc_middle/src/ty/trait_def.rs b/compiler/rustc_middle/src/ty/trait_def.rs
@@ -53,12 +53,14 @@ pub struct TraitDef {
     /// must be implemented.
     pub must_implement_one_of: Option<Box<[Ident]>>,
 
-    /// Whether a type's built-in `dyn Trait: Trait` implementation is explicitly
-    /// denied. This only applies to built-in trait, and is marked via
-    /// `#[rustc_do_not_implement_via_object]`.
-    pub do_not_implement_via_object: bool,
+    /// 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,
 }
 

diff --git a/compiler/rustc_passes/messages.ftl b/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

diff --git a/compiler/rustc_passes/src/check_attr.rs b/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 => {

diff --git a/compiler/rustc_passes/src/errors.rs b/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 {

diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -817,6 +817,7 @@ symbols! {
         impl_trait_in_bindings,
         impl_trait_in_fn_trait_return,
         impl_trait_projections,
+        implement_via_object,
         implied_by,
         import,
         import_name_type,
@@ -1270,7 +1271,6 @@ symbols! {
         rustc_diagnostic_macros,
         rustc_dirty,
         rustc_do_not_const_check,
-        rustc_do_not_implement_via_object,
         rustc_doc_primitive,
         rustc_dummy,
         rustc_dump_env_program_clauses,

diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs
@@ -1586,6 +1586,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() {

diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
@@ -554,6 +554,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             "assemble_candidates_from_object_ty",
         );
 
+        if !self.tcx().trait_def(obligation.predicate.def_id()).implement_via_object {
+            return;
+        }
+
         self.infcx.probe(|_snapshot| {
             if obligation.has_non_region_late_bound() {
                 return;

diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs
@@ -140,7 +140,8 @@ unsafe impl<T: Sync + ?Sized> Send for &T {}
 )]
 #[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable
 #[rustc_specialization_trait]
-#[rustc_deny_explicit_impl]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 #[rustc_coinductive]
 pub trait Sized {
     // Empty.
@@ -173,8 +174,8 @@ pub trait Sized {
 /// [nomicon-coerce]: ../../nomicon/coercions.html
 #[unstable(feature = "unsize", issue = "18598")]
 #[lang = "unsize"]
-#[rustc_deny_explicit_impl]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub trait Unsize<T: ?Sized> {
     // Empty.
 }
@@ -855,8 +856,8 @@ impl<T: ?Sized> StructuralEq for PhantomData<T> {}
     reason = "this trait is unlikely to ever be stabilized, use `mem::discriminant` instead"
 )]
 #[lang = "discriminant_kind"]
-#[rustc_deny_explicit_impl]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub trait DiscriminantKind {
     /// The type of the discriminant, which must satisfy the trait
     /// bounds required by `mem::Discriminant`.
@@ -961,8 +962,8 @@ marker_impls! {
 #[unstable(feature = "const_trait_impl", issue = "67792")]
 #[lang = "destruct"]
 #[rustc_on_unimplemented(message = "can't drop `{Self}`", append_const_msg)]
-#[rustc_deny_explicit_impl]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 #[const_trait]
 pub trait Destruct {}
 
@@ -973,8 +974,8 @@ pub trait Destruct {}
 #[unstable(feature = "tuple_trait", issue = "none")]
 #[lang = "tuple_trait"]
 #[rustc_on_unimplemented(message = "`{Self}` is not a tuple")]
-#[rustc_deny_explicit_impl]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub trait Tuple {}
 
 /// A marker for pointer-like types.
@@ -1029,7 +1030,8 @@ impl ConstParamTy for () {}
     reason = "internal trait for implementing various traits for all function pointers"
 )]
 #[lang = "fn_ptr_trait"]
-#[rustc_deny_explicit_impl]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub trait FnPtr: Copy + Clone {
     /// Returns the address of the function pointer.
     #[lang = "fn_ptr_addr"]

diff --git a/library/core/src/mem/transmutability.rs b/library/core/src/mem/transmutability.rs
@@ -7,7 +7,8 @@ use crate::marker::ConstParamTy;
 /// notwithstanding whatever safety checks you have asked the compiler to [`Assume`] are satisfied.
 #[unstable(feature = "transmutability", issue = "99571")]
 #[lang = "transmute_trait"]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub unsafe trait BikeshedIntrinsicFrom<Src, Context, const ASSUME: Assume = { Assume::NOTHING }>
 where
     Src: ?Sized,

diff --git a/library/core/src/ptr/metadata.rs b/library/core/src/ptr/metadata.rs
@@ -50,8 +50,8 @@ use crate::hash::{Hash, Hasher};
 ///
 /// [`to_raw_parts`]: *const::to_raw_parts
 #[lang = "pointee_trait"]
-#[rustc_deny_explicit_impl]
-#[cfg_attr(not(bootstrap), rustc_do_not_implement_via_object)]
+#[cfg_attr(not(bootstrap), rustc_deny_explicit_impl(implement_via_object = false))]
+#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
 pub trait Pointee {
     /// The type for metadata in pointers and references to `Self`.
     #[lang = "metadata_type"]

diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/attr-misuse.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/attr-misuse.stderr
@@ -3,12 +3,16 @@ error: attribute should be applied to a trait
    |
 LL | #[const_trait]
    | ^^^^^^^^^^^^^^
+LL | fn main() {}
+   | ------------ not a trait
 
 error: attribute should be applied to a trait
   --> $DIR/attr-misuse.rs:5:5
    |
 LL |     #[const_trait]
    |     ^^^^^^^^^^^^^^
+LL |     fn foo(self);
+   |     ------------- not a trait
 
 error: aborting due to 2 previous errors
 
diff --git a/tests/ui/traits/deny-builtin-object-impl.current.stderr b/tests/ui/traits/deny-builtin-object-impl.current.stderr
@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `dyn NotObject: NotObject` is not satisfied
+  --> $DIR/deny-builtin-object-impl.rs:18:23
+   |
+LL |     test_not_object::<dyn NotObject>();
+   |                       ^^^^^^^^^^^^^ the trait `NotObject` is not implemented for `dyn NotObject`
+   |
+note: required by a bound in `test_not_object`
+  --> $DIR/deny-builtin-object-impl.rs:14:23
+   |
+LL | fn test_not_object<T: NotObject + ?Sized>() {}
+   |                       ^^^^^^^^^ required by this bound in `test_not_object`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/traits/deny-builtin-object-impl.next.stderr b/tests/ui/traits/deny-builtin-object-impl.next.stderr
@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `dyn NotObject: NotObject` is not satisfied
+  --> $DIR/deny-builtin-object-impl.rs:18:23
+   |
+LL |     test_not_object::<dyn NotObject>();
+   |                       ^^^^^^^^^^^^^ the trait `NotObject` is not implemented for `dyn NotObject`
+   |
+note: required by a bound in `test_not_object`
+  --> $DIR/deny-builtin-object-impl.rs:14:23
+   |
+LL | fn test_not_object<T: NotObject + ?Sized>() {}
+   |                       ^^^^^^^^^ required by this bound in `test_not_object`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/traits/deny-builtin-object-impl.rs b/tests/ui/traits/deny-builtin-object-impl.rs
@@ -0,0 +1,20 @@
+// revisions: current next
+//[next] compile-flags: -Ztrait-solver=next
+
+#![feature(rustc_attrs)]
+
+#[rustc_deny_explicit_impl(implement_via_object = true)]
+trait YesObject {}
+
+#[rustc_deny_explicit_impl(implement_via_object = false)]
+trait NotObject {}
+
+fn test_yes_object<T: YesObject + ?Sized>() {}
+
+fn test_not_object<T: NotObject + ?Sized>() {}
+
+fn main() {
+    test_yes_object::<dyn YesObject>();
+    test_not_object::<dyn NotObject>();
+    //~^ ERROR the trait bound `dyn NotObject: NotObject` is not satisfied
+}