Use a trait to enforce field validity for union fields + unsafe fields + unsafe<> binder types

[compiler-errors]

This PR introduces a new, internal-only trait called BikeshedGuaranteedNoDrop¹ to faithfully model the field check that used to be implemented manually by allowed_union_or_unsafe_field.

rust/compiler/rustc_hir_analysis/src/check/check.rs

Lines 84 to 115 in 942db67

	fn allowed_union_or_unsafe_field<'tcx>(
	tcx: TyCtxt<'tcx>,
	ty: Ty<'tcx>,
	typing_env: ty::TypingEnv<'tcx>,
	span: Span,
	) -> bool {
	// We don't just accept all !needs_drop fields, due to semver concerns.
	let allowed = match ty.kind() {
	ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
	ty::Tuple(tys) => {
	// allow tuples of allowed types
	tys.iter().all(|ty| allowed_union_or_unsafe_field(tcx, ty, typing_env, span))
	}
	ty::Array(elem, _len) => {
	// Like `Copy`, we do *not* special-case length 0.
	allowed_union_or_unsafe_field(tcx, *elem, typing_env, span)
	}
	_ => {
	// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
	// also no need to report an error if the type is unresolved.
	ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
	|| tcx.type_is_copy_modulo_regions(typing_env, ty)
	|| ty.references_error()
	}
	};
	if allowed && ty.needs_drop(tcx, typing_env) {
	// This should never happen. But we can get here e.g. in case of name resolution errors.
	tcx.dcx()
	.span_delayed_bug(span, "we should never accept maybe-dropping union or unsafe fields");
	}
	allowed
	}

Copying over the doc comment from the trait:

/// Marker trait for the types that are allowed in union fields, unsafe fields,
/// and unsafe binder types.
///
/// Implemented for:
/// * `&T`, `&mut T` for all `T`,
/// * `ManuallyDrop<T>` for all `T`,
/// * tuples and arrays whose elements implement `BikeshedGuaranteedNoDrop`,
/// * or otherwise, all types that are `Copy`.
///
/// Notably, this doesn't include all trivially-destructible types for semver
/// reasons.
///
/// Bikeshed name for now.

As far as I am aware, there's no new behavior being guaranteed by this trait, since it operates the same as the manually implemented check. We could easily rip out this trait and go back to using the manually implemented check for union fields, however using a trait means that this code can be shared by WF for unsafe<> binders too. See the last commit.

The only diagnostic changes are that this now fires false-negatives for fields that are ill-formed. I don't consider that to be much of a problem though.

r? oli-obk

Footnotes

1. Please let's not bikeshed this name lol. There's no good name for ValidForUnsafeFieldsUnsafeBindersAndUnionFields. ↩

[rustbot]

changes to the core type system

cc @compiler-errors, @lcnr

[compiler-errors]

There's a lot of random fixes in this file to make it compile lol

[compiler-errors]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 885f835 with merge 0a914ae...

[bors]

☀️ Try build successful - checks-actions
Build commit: 0a914ae (0a914aea5a53390f79db9a8f7c4d6848cd98bd3e)

[rust-timer]

Finished benchmarking commit (0a914ae): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary 1.1%, secondary 2.2%)

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)	1.1%	[0.9%, 1.4%]	2
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[0.9%, 1.4%]	2

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: 780.473s -> 780.252s (-0.03%)
Artifact size: 329.02 MiB -> 329.03 MiB (0.00%)

[lcnr]

nit, otherwise r=me

[compiler-errors]

Regarding the codegen tests (which has a surprising number of no_core tests that don't use minicore stubs, and which changing all of them is a headache) -- I'm gonna modify this to fall back to the Copy lang item if the BikeshedGuaranteedNoDrop lang item is missing.

[rustbot]

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[compiler-errors]

@bors r=lcnr

[bors]

📌 Commit 5c67cfa has been approved by lcnr

It is now in the queue for this repository.

[Noratrieb]

@bors rollup=maybe perf neutral

[jhpratt]

@bors r- rollup=iffy

failing tidy in #136952

[compiler-errors]

@bors r=lcnr rollup=maybe

[bors]

📌 Commit d0564fd has been approved by lcnr

It is now in the queue for this repository.