Skip to content

Commit

Permalink
Auto merge of #112320 - compiler-errors:do-not-impl-via-obj, r=lcnr
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Jun 20, 2023
2 parents b9d608c + ca68cf0 commit 6fc0273
Show file tree
Hide file tree
Showing 21 changed files with 175 additions and 60 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_hir_analysis/src/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
46 changes: 46 additions & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
}
}

Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 6 additions & 37 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
_ => {
Expand Down Expand Up @@ -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 => {
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,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;
Expand Down
18 changes: 12 additions & 6 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -173,7 +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_deny_explicit_impl(implement_via_object = false))]
#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
pub trait Unsize<T: ?Sized> {
// Empty.
}
Expand Down Expand Up @@ -854,7 +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_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`.
Expand Down Expand Up @@ -959,7 +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_deny_explicit_impl(implement_via_object = false))]
#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
#[const_trait]
pub trait Destruct {}

Expand All @@ -970,7 +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_deny_explicit_impl(implement_via_object = false))]
#[cfg_attr(bootstrap, rustc_deny_explicit_impl)]
pub trait Tuple {}

/// A marker for pointer-like types.
Expand Down Expand Up @@ -1025,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"]
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/mem/transmutability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +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_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,
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/ptr/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +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_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"]
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/rfcs/rfc-2632-const-trait-impl/attr-misuse.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

15 changes: 15 additions & 0 deletions tests/ui/traits/deny-builtin-object-impl.current.stderr
Original file line number Diff line number Diff line change
@@ -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`.
15 changes: 15 additions & 0 deletions tests/ui/traits/deny-builtin-object-impl.next.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit 6fc0273

Please sign in to comment.