Add implement_via_object to rustc_deny_explicit_impl to control object candidate assembly

[compiler-errors]

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:

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

Lines 61 to 132 in 2f896da

	} else {
	self.assemble_candidates_for_trait_alias(obligation, &mut candidates);
	// Other bounds. Consider both in-scope bounds from fn decl
	// and applicable impls. There is a certain set of precedence rules here.
	let def_id = obligation.predicate.def_id();
	let lang_items = self.tcx().lang_items();
	if lang_items.copy_trait() == Some(def_id) {
	debug!(obligation_self_ty = ?obligation.predicate.skip_binder().self_ty());
	// User-defined copy impls are permitted, but only for
	// structs and enums.
	self.assemble_candidates_from_impls(obligation, &mut candidates);
	// For other types, we'll use the builtin rules.
	let copy_conditions = self.copy_clone_conditions(obligation);
	self.assemble_builtin_bound_candidates(copy_conditions, &mut candidates);
	} else if lang_items.discriminant_kind_trait() == Some(def_id) {
	// `DiscriminantKind` is automatically implemented for every type.
	candidates.vec.push(BuiltinCandidate { has_nested: false });
	} else if lang_items.pointee_trait() == Some(def_id) {
	// `Pointee` is automatically implemented for every type.
	candidates.vec.push(BuiltinCandidate { has_nested: false });
	} else if lang_items.sized_trait() == Some(def_id) {
	// Sized is never implementable by end-users, it is
	// always automatically computed.
	let sized_conditions = self.sized_conditions(obligation);
	self.assemble_builtin_bound_candidates(sized_conditions, &mut candidates);
	} else if lang_items.unsize_trait() == Some(def_id) {
	self.assemble_candidates_for_unsizing(obligation, &mut candidates);
	} else if lang_items.destruct_trait() == Some(def_id) {
	self.assemble_const_destruct_candidates(obligation, &mut candidates);
	} else if lang_items.transmute_trait() == Some(def_id) {
	// User-defined transmutability impls are permitted.
	self.assemble_candidates_from_impls(obligation, &mut candidates);
	self.assemble_candidates_for_transmutability(obligation, &mut candidates);
	} else if lang_items.tuple_trait() == Some(def_id) {
	self.assemble_candidate_for_tuple(obligation, &mut candidates);
	} else if lang_items.pointer_like() == Some(def_id) {
	self.assemble_candidate_for_pointer_like(obligation, &mut candidates);
	} else if lang_items.fn_ptr_trait() == Some(def_id) {
	self.assemble_candidates_for_fn_ptr_trait(obligation, &mut candidates);
	} else {
	if lang_items.clone_trait() == Some(def_id) {
	// Same builtin conditions as `Copy`, i.e., every type which has builtin support
	// for `Copy` also has builtin support for `Clone`, and tuples/arrays of `Clone`
	// types have builtin support for `Clone`.
	let clone_conditions = self.copy_clone_conditions(obligation);
	self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates);
	}
	if lang_items.gen_trait() == Some(def_id) {
	self.assemble_generator_candidates(obligation, &mut candidates);
	} else if lang_items.future_trait() == Some(def_id) {
	self.assemble_future_candidates(obligation, &mut candidates);
	}
	self.assemble_closure_candidates(obligation, &mut candidates);
	self.assemble_fn_pointer_candidates(obligation, &mut candidates);
	self.assemble_candidates_from_impls(obligation, &mut candidates);
	self.assemble_candidates_from_object_ty(obligation, &mut candidates);
	}
	self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
	self.assemble_candidates_from_caller_bounds(stack, &mut candidates)?;
	// Auto implementations have lower priority, so we only
	// consider triggering a default if there is no other impl that can apply.
	if candidates.vec.is_empty() {
	self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
	}
	}

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

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[lcnr]

as a thought, do we want to change this to instead add a argument to rustc_deny_explicit_impl, i.e. #[rustc_deny_explicit_impl(builtin_object_impl = <bool>)] 🤔

going with opt-out here feels dangerous to me as getting it wrong is unsound(?), so maybe alternatively change it to #[rustc_builtin_has_object_impl]? 🤔

[lcnr]

please instead add another field to TraitDef and access that here. you can then mark rustc_do_not_implement_via_object as @only_local.

[compiler-errors]

as a thought, do we want to change this to instead add a argument to rustc_deny_explicit_impl, i.e. #[rustc_deny_explicit_impl(builtin_object_impl = )] 🤔

I don't think we can do that, since the transmutability trait BikeshedIntrinsicFrom, I think will allow custom implementations in order to indicate that a type is transmutable to another trait... 🤔 Maybe I'm wrong, though, cc @jswrenn / @bryangarza.

[lcnr]

pls add check to rustc_passes/src/check_attr.rs, after that r=me

[bryangarza]

as a thought, do we want to change this to instead add a argument to rustc_deny_explicit_impl, i.e. #[rustc_deny_explicit_impl(builtin_object_impl = )] 🤔

I don't think we can do that, since the transmutability trait BikeshedIntrinsicFrom, I think will allow custom implementations in order to indicate that a type is transmutable to another trait... 🤔 Maybe I'm wrong, though, cc @jswrenn / @bryangarza.

BikeshedIntrinsicFrom is not meant to be implemented by end-users, I think the only way they can use it is by using it as as trait bound, and then the trait solver will check that BikeshedIntrinsicFrom<Src> is implemented for Dst

If you're asking about custom implementations in rustc itself... I can check with @jswrenn offline and get back to you

[compiler-errors]

Well, given that BikeshedIntrinsicFrom is not intended to be implemented (afaict, could change later I guess, it is an unsafe trait for a reason perhaps?) I just made this an attribute on rustc_deny_explicit_impl.

Maybe we could uplift this into something like rustc_builtin_impl(deny_explicit_impl = true, deny_builtin_object_impl = true) but I'm too lazy to do that, could be convinced otherwise tho I guess 😸

[lcnr]

r=me after nits and updating PR descr + title

[compiler-errors]

@bors r=lcnr

[bors]

📌 Commit ca68cf0 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit ca68cf0 with merge 6fc0273...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 6fc0273 to master...

[rust-timer]

Finished benchmarking commit (6fc0273): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.6%	[0.6%, 0.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 658.605s -> 655.831s (-0.42%)