From ca68cf0d46ec3485be177eeec3f4a4662b21bc8c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 16 Jun 2023 23:45:01 +0000 Subject: [PATCH] Merge attrs, better validation --- compiler/rustc_feature/src/builtin_attrs.rs | 11 ++--- compiler/rustc_hir_analysis/src/collect.rs | 46 ++++++++++++++++++- compiler/rustc_middle/src/ty/trait_def.rs | 10 ++-- compiler/rustc_passes/messages.ftl | 3 -- compiler/rustc_passes/src/check_attr.rs | 43 +++-------------- compiler/rustc_passes/src/errors.rs | 7 --- compiler/rustc_span/src/symbol.rs | 2 +- .../src/traits/project.rs | 4 ++ .../src/traits/select/candidate_assembly.rs | 4 ++ library/core/src/marker.rs | 22 +++++---- library/core/src/mem/transmutability.rs | 3 +- library/core/src/ptr/metadata.rs | 4 +- .../attr-misuse.stderr | 4 ++ .../deny-builtin-object-impl.current.stderr | 15 ++++++ .../deny-builtin-object-impl.next.stderr | 15 ++++++ tests/ui/traits/deny-builtin-object-impl.rs | 20 ++++++++ 16 files changed, 140 insertions(+), 73 deletions(-) create mode 100644 tests/ui/traits/deny-builtin-object-impl.current.stderr create mode 100644 tests/ui/traits/deny-builtin-object-impl.next.stderr create mode 100644 tests/ui/traits/deny-builtin-object-impl.rs diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 0e7deef0cff..3c5bff3812a 100644 --- 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 index 17a180a7eb2..ec378c0984f 100644 --- 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 index 325e156fba5..98c70e330f1 100644 --- 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>, - /// 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 index e76f1614b93..a607e483c97 100644 --- 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 index c35c7da2664..073760f394e 100644 --- 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 index ae624dbc9c9..7890c93d5ff 100644 --- 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 index f476f891631..cd1a16a1b89 100644 --- 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 index 563cc257e03..5554c8a0cc4 100644 --- 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 index 3c223db5a0b..9fb9385eefc 100644 --- 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 index e1359aa09a2..760e58276fc 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -140,7 +140,8 @@ unsafe impl 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 { // Empty. } @@ -855,8 +856,8 @@ impl StructuralEq for PhantomData {} 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 index 8ba4313cc1e..3805d149b70 100644 --- 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 where Src: ?Sized, diff --git a/library/core/src/ptr/metadata.rs b/library/core/src/ptr/metadata.rs index 34f31fb9fc6..daaa44b1d9a 100644 --- 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 index b18f33218c2..998958cedf7 100644 --- 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 new file mode 100644 index 00000000000..5c1987426f7 --- /dev/null +++ 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::(); + | ^^^^^^^^^^^^^ 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() {} + | ^^^^^^^^^ 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 new file mode 100644 index 00000000000..5c1987426f7 --- /dev/null +++ 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::(); + | ^^^^^^^^^^^^^ 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() {} + | ^^^^^^^^^ 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 new file mode 100644 index 00000000000..dce03a43b68 --- /dev/null +++ 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() {} + +fn test_not_object() {} + +fn main() { + test_yes_object::(); + test_not_object::(); + //~^ ERROR the trait bound `dyn NotObject: NotObject` is not satisfied +}