Rollup merge of rust-lang#136660 - compiler-errors:BikeshedGuaranteedNoDrop, r=lcnr

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

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

https://github.com/rust-lang/rust/blob/942db6782f4a28c55b0b75b38fd4394d0483390f/compiler/rustc_hir_analysis/src/check/check.rs#L84-L115

Copying over the doc comment from the trait:

```rust
/// 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

[^1]: Please let's not bikeshed this name lol. There's no good name for `ValidForUnsafeFieldsUnsafeBindersAndUnionFields`.

Author: jhpratt

Diff for: compiler/rustc_codegen_cranelift/example/mini_core.rs

@@ -57,6 +57,9 @@ impl<T: ?Sized> LegacyReceiver for Box<T> {}
 #[lang = "copy"]
 pub trait Copy {}
 
+#[lang = "bikeshed_guaranteed_no_drop"]
+pub trait BikeshedGuaranteedNoDrop {}
+
 impl Copy for bool {}
 impl Copy for u8 {}
 impl Copy for u16 {}

Diff for: compiler/rustc_codegen_gcc/example/mini_core.rs

@@ -54,6 +54,9 @@ impl<T: ?Sized, A: Allocator> LegacyReceiver for Box<T, A> {}
 #[lang = "copy"]
 pub trait Copy {}
 
+#[lang = "bikeshed_guaranteed_no_drop"]
+pub trait BikeshedGuaranteedNoDrop {}
+
 impl Copy for bool {}
 impl Copy for u8 {}
 impl Copy for u16 {}

Diff for: compiler/rustc_hir/src/lang_items.rs

@@ -351,6 +351,7 @@ language_item_table! {
     PhantomData,             sym::phantom_data,        phantom_data,               Target::Struct,         GenericRequirement::Exact(1);
 
     ManuallyDrop,            sym::manually_drop,       manually_drop,              Target::Struct,         GenericRequirement::None;
+    BikeshedGuaranteedNoDrop, sym::bikeshed_guaranteed_no_drop, bikeshed_guaranteed_no_drop, Target::Trait, GenericRequirement::Exact(0);
 
     MaybeUninit,             sym::maybe_uninit,        maybe_uninit,               Target::Union,          GenericRequirement::None;
 

Diff for: compiler/rustc_hir_analysis/src/check/check.rs

@@ -6,7 +6,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_errors::MultiSpan;
 use rustc_errors::codes::*;
 use rustc_hir::def::{CtorKind, DefKind};
-use rustc_hir::{Node, intravisit};
+use rustc_hir::{LangItem, Node, intravisit};
 use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
 use rustc_infer::traits::{Obligation, ObligationCauseCode};
 use rustc_lint_defs::builtin::{
@@ -27,6 +27,7 @@ use rustc_session::lint::builtin::UNINHABITED_STATIC;
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::error_reporting::traits::on_unimplemented::OnUnimplementedDirective;
 use rustc_trait_selection::traits;
+use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
 use rustc_type_ir::fold::TypeFoldable;
 use tracing::{debug, instrument};
 use ty::TypingMode;
@@ -87,89 +88,76 @@ fn allowed_union_or_unsafe_field<'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");
+    // HACK (not that bad of a hack don't worry): Some codegen tests don't even define proper
+    // impls for `Copy`. Let's short-circuit here for this validity check, since a lot of them
+    // use unions. We should eventually fix all the tests to define that lang item or use
+    // minicore stubs.
+    if ty.is_trivially_pure_clone_copy() {
+        return true;
     }
-    allowed
+    // If `BikeshedGuaranteedNoDrop` is not defined in a `#[no_core]` test, fall back to `Copy`.
+    // This is an underapproximation of `BikeshedGuaranteedNoDrop`,
+    let def_id = tcx
+        .lang_items()
+        .get(LangItem::BikeshedGuaranteedNoDrop)
+        .unwrap_or_else(|| tcx.require_lang_item(LangItem::Copy, Some(span)));
+    let Ok(ty) = tcx.try_normalize_erasing_regions(typing_env, ty) else {
+        tcx.dcx().span_delayed_bug(span, "could not normalize field type");
+        return true;
+    };
+    let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
+    infcx.predicate_must_hold_modulo_regions(&Obligation::new(
+        tcx,
+        ObligationCause::dummy_with_span(span),
+        param_env,
+        ty::TraitRef::new(tcx, def_id, [ty]),
+    ))
 }
 
 /// Check that the fields of the `union` do not need dropping.
 fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> bool {
-    let item_type = tcx.type_of(item_def_id).instantiate_identity();
-    if let ty::Adt(def, args) = item_type.kind() {
-        assert!(def.is_union());
-
-        let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
-        for field in &def.non_enum_variant().fields {
-            let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
-            else {
-                tcx.dcx().span_delayed_bug(span, "could not normalize field type");
-                continue;
-            };
+    let def = tcx.adt_def(item_def_id);
+    assert!(def.is_union());
 
-            if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
-                let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
-                    // We are currently checking the type this field came from, so it must be local.
-                    Some(Node::Field(field)) => (field.span, field.ty.span),
-                    _ => unreachable!("mir field has to correspond to hir field"),
-                };
-                tcx.dcx().emit_err(errors::InvalidUnionField {
-                    field_span,
-                    sugg: errors::InvalidUnionFieldSuggestion {
-                        lo: ty_span.shrink_to_lo(),
-                        hi: ty_span.shrink_to_hi(),
-                    },
-                    note: (),
-                });
-                return false;
-            }
+    let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
+    let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
+
+    for field in &def.non_enum_variant().fields {
+        if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
+            let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
+                // We are currently checking the type this field came from, so it must be local.
+                Some(Node::Field(field)) => (field.span, field.ty.span),
+                _ => unreachable!("mir field has to correspond to hir field"),
+            };
+            tcx.dcx().emit_err(errors::InvalidUnionField {
+                field_span,
+                sugg: errors::InvalidUnionFieldSuggestion {
+                    lo: ty_span.shrink_to_lo(),
+                    hi: ty_span.shrink_to_hi(),
+                },
+                note: (),
+            });
+            return false;
         }
-    } else {
-        span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind());
     }
+
     true
 }
 
 /// Check that the unsafe fields do not need dropping.
 fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
     let span = tcx.def_span(item_def_id);
-    let item_type = tcx.type_of(item_def_id).instantiate_identity();
-    let ty::Adt(def, args) = item_type.kind() else {
-        span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
-    };
+    let def = tcx.adt_def(item_def_id);
+
     let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
+    let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
+
     for field in def.all_fields() {
         if !field.safety.is_unsafe() {
             continue;
         }
-        let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
-        else {
-            tcx.dcx().span_delayed_bug(span, "could not normalize field type");
-            continue;
-        };
 
-        if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
+        if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
             let hir::Node::Field(field) = tcx.hir_node_by_def_id(field.did.expect_local()) else {
                 unreachable!("field has to correspond to hir field")
             };

Diff for: compiler/rustc_middle/src/traits/select.rs

@@ -168,6 +168,8 @@ pub enum SelectionCandidate<'tcx> {
     BuiltinObjectCandidate,
 
     BuiltinUnsizeCandidate,
+
+    BikeshedGuaranteedNoDropCandidate,
 }
 
 /// The result of trait evaluation. The order is important

Diff for: compiler/rustc_middle/src/ty/adt.rs

@@ -217,6 +217,10 @@ impl<'tcx> rustc_type_ir::inherent::AdtDef<TyCtxt<'tcx>> for AdtDef<'tcx> {
         self.is_phantom_data()
     }
 
+    fn is_manually_drop(self) -> bool {
+        self.is_manually_drop()
+    }
+
     fn all_field_tys(
         self,
         tcx: TyCtxt<'tcx>,

Diff for: compiler/rustc_middle/src/ty/context.rs

@@ -690,6 +690,7 @@ bidirectional_lang_item_map! {
     AsyncFnOnce,
     AsyncFnOnceOutput,
     AsyncIterator,
+    BikeshedGuaranteedNoDrop,
     CallOnceFuture,
     CallRefFuture,
     Clone,

Diff for: compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -267,6 +267,11 @@ where
         goal: Goal<I, Self>,
     ) -> Result<Candidate<I>, NoSolution>;
 
+    fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
+        ecx: &mut EvalCtxt<'_, D>,
+        goal: Goal<I, Self>,
+    ) -> Result<Candidate<I>, NoSolution>;
+
     /// Consider (possibly several) candidates to upcast or unsize a type to another
     /// type, excluding the coercion of a sized type into a `dyn Trait`.
     ///
@@ -478,6 +483,9 @@ where
                 Some(TraitSolverLangItem::TransmuteTrait) => {
                     G::consider_builtin_transmute_candidate(self, goal)
                 }
+                Some(TraitSolverLangItem::BikeshedGuaranteedNoDrop) => {
+                    G::consider_builtin_bikeshed_guaranteed_no_drop_candidate(self, goal)
+                }
                 _ => Err(NoSolution),
             }
         };

Diff for: compiler/rustc_next_trait_solver/src/solve/effect_goals.rs

@@ -374,6 +374,13 @@ where
         unreachable!("TransmuteFrom is not const")
     }
 
+    fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
+        _ecx: &mut EvalCtxt<'_, D>,
+        _goal: Goal<I, Self>,
+    ) -> Result<Candidate<I>, NoSolution> {
+        unreachable!("BikeshedGuaranteedNoDrop is not const");
+    }
+
     fn consider_structural_builtin_unsize_candidates(
         _ecx: &mut EvalCtxt<'_, D>,
         _goal: Goal<I, Self>,

Diff for: compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs

@@ -942,6 +942,13 @@ where
     ) -> Result<Candidate<I>, NoSolution> {
         panic!("`TransmuteFrom` does not have an associated type: {:?}", goal)
     }
+
+    fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
+        _ecx: &mut EvalCtxt<'_, D>,
+        goal: Goal<I, Self>,
+    ) -> Result<Candidate<I>, NoSolution> {
+        unreachable!("`BikeshedGuaranteedNoDrop` does not have an associated type: {:?}", goal)
+    }
 }
 
 impl<D, I> EvalCtxt<'_, D>

Diff for: compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

@@ -625,6 +625,101 @@ where
         })
     }
 
+    /// NOTE: This is implemented as a built-in goal and not a set of impls like:
+    ///
+    /// ```rust,ignore (illustrative)
+    /// impl<T> BikeshedGuaranteedNoDrop for T where T: Copy {}
+    /// impl<T> BikeshedGuaranteedNoDrop for ManuallyDrop<T> {}
+    /// ```
+    ///
+    /// because these impls overlap, and I'd rather not build a coherence hack for
+    /// this harmless overlap.
+    fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
+        ecx: &mut EvalCtxt<'_, D>,
+        goal: Goal<I, Self>,
+    ) -> Result<Candidate<I>, NoSolution> {
+        if goal.predicate.polarity != ty::PredicatePolarity::Positive {
+            return Err(NoSolution);
+        }
+
+        let cx = ecx.cx();
+        ecx.probe_builtin_trait_candidate(BuiltinImplSource::Misc).enter(|ecx| {
+            let ty = goal.predicate.self_ty();
+            match ty.kind() {
+                // `&mut T` and `&T` always implement `BikeshedGuaranteedNoDrop`.
+                ty::Ref(..) => {}
+                // `ManuallyDrop<T>` always implements `BikeshedGuaranteedNoDrop`.
+                ty::Adt(def, _) if def.is_manually_drop() => {}
+                // Arrays and tuples implement `BikeshedGuaranteedNoDrop` only if
+                // their constituent types implement `BikeshedGuaranteedNoDrop`.
+                ty::Tuple(tys) => {
+                    ecx.add_goals(
+                        GoalSource::ImplWhereBound,
+                        tys.iter().map(|elem_ty| {
+                            goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty]))
+                        }),
+                    );
+                }
+                ty::Array(elem_ty, _) => {
+                    ecx.add_goal(
+                        GoalSource::ImplWhereBound,
+                        goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty])),
+                    );
+                }
+
+                // All other types implement `BikeshedGuaranteedNoDrop` only if
+                // they implement `Copy`. We could be smart here and short-circuit
+                // some trivially `Copy`/`!Copy` types, but there's no benefit.
+                ty::FnDef(..)
+                | ty::FnPtr(..)
+                | ty::Error(_)
+                | ty::Uint(_)
+                | ty::Int(_)
+                | ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
+                | ty::Bool
+                | ty::Float(_)
+                | ty::Char
+                | ty::RawPtr(..)
+                | ty::Never
+                | ty::Pat(..)
+                | ty::Dynamic(..)
+                | ty::Str
+                | ty::Slice(_)
+                | ty::Foreign(..)
+                | ty::Adt(..)
+                | ty::Alias(..)
+                | ty::Param(_)
+                | ty::Placeholder(..)
+                | ty::Closure(..)
+                | ty::CoroutineClosure(..)
+                | ty::Coroutine(..)
+                | ty::UnsafeBinder(_)
+                | ty::CoroutineWitness(..) => {
+                    ecx.add_goal(
+                        GoalSource::ImplWhereBound,
+                        goal.with(
+                            cx,
+                            ty::TraitRef::new(
+                                cx,
+                                cx.require_lang_item(TraitSolverLangItem::Copy),
+                                [ty],
+                            ),
+                        ),
+                    );
+                }
+
+                ty::Bound(..)
+                | ty::Infer(
+                    ty::TyVar(_) | ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_),
+                ) => {
+                    panic!("unexpected type `{ty:?}`")
+                }
+            }
+
+            ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+        })
+    }
+
     /// ```ignore (builtin impl example)
     /// trait Trait {
     ///     fn foo(&self);

Diff for: compiler/rustc_span/src/symbol.rs

@@ -516,6 +516,7 @@ symbols! {
         bang,
         begin_panic,
         bench,
+        bikeshed_guaranteed_no_drop,
         bin,
         binaryheap_iter,
         bind_by_move_pattern_guards,