Skip to content

Commit 516afd5

Browse files
Implement and use BikeshedGuaranteedNoDrop for union/unsafe field validity
1 parent c72d443 commit 516afd5

File tree

21 files changed

+366
-67
lines changed

21 files changed

+366
-67
lines changed

compiler/rustc_codegen_cranelift/example/mini_core.rs

+3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ impl<T: ?Sized> LegacyReceiver for Box<T> {}
5757
#[lang = "copy"]
5858
pub trait Copy {}
5959

60+
#[lang = "bikeshed_guaranteed_no_drop"]
61+
pub trait BikeshedGuaranteedNoDrop {}
62+
6063
impl Copy for bool {}
6164
impl Copy for u8 {}
6265
impl Copy for u16 {}

compiler/rustc_codegen_gcc/example/mini_core.rs

+3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ impl<T: ?Sized, A: Allocator> LegacyReceiver for Box<T, A> {}
5454
#[lang = "copy"]
5555
pub trait Copy {}
5656

57+
#[lang = "bikeshed_guaranteed_no_drop"]
58+
pub trait BikeshedGuaranteedNoDrop {}
59+
5760
impl Copy for bool {}
5861
impl Copy for u8 {}
5962
impl Copy for u16 {}

compiler/rustc_hir/src/lang_items.rs

+1
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ language_item_table! {
351351
PhantomData, sym::phantom_data, phantom_data, Target::Struct, GenericRequirement::Exact(1);
352352

353353
ManuallyDrop, sym::manually_drop, manually_drop, Target::Struct, GenericRequirement::None;
354+
BikeshedGuaranteedNoDrop, sym::bikeshed_guaranteed_no_drop, bikeshed_guaranteed_no_drop, Target::Trait, GenericRequirement::Exact(0);
354355

355356
MaybeUninit, sym::maybe_uninit, maybe_uninit, Target::Union, GenericRequirement::None;
356357

compiler/rustc_hir_analysis/src/check/check.rs

+52-64
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
66
use rustc_errors::MultiSpan;
77
use rustc_errors::codes::*;
88
use rustc_hir::def::{CtorKind, DefKind};
9-
use rustc_hir::{Node, intravisit};
9+
use rustc_hir::{LangItem, Node, intravisit};
1010
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
1111
use rustc_infer::traits::{Obligation, ObligationCauseCode};
1212
use rustc_lint_defs::builtin::{
@@ -27,6 +27,7 @@ use rustc_session::lint::builtin::UNINHABITED_STATIC;
2727
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
2828
use rustc_trait_selection::error_reporting::traits::on_unimplemented::OnUnimplementedDirective;
2929
use rustc_trait_selection::traits;
30+
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
3031
use rustc_type_ir::fold::TypeFoldable;
3132
use tracing::{debug, instrument};
3233
use ty::TypingMode;
@@ -87,89 +88,76 @@ fn allowed_union_or_unsafe_field<'tcx>(
8788
typing_env: ty::TypingEnv<'tcx>,
8889
span: Span,
8990
) -> bool {
90-
// We don't just accept all !needs_drop fields, due to semver concerns.
91-
let allowed = match ty.kind() {
92-
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
93-
ty::Tuple(tys) => {
94-
// allow tuples of allowed types
95-
tys.iter().all(|ty| allowed_union_or_unsafe_field(tcx, ty, typing_env, span))
96-
}
97-
ty::Array(elem, _len) => {
98-
// Like `Copy`, we do *not* special-case length 0.
99-
allowed_union_or_unsafe_field(tcx, *elem, typing_env, span)
100-
}
101-
_ => {
102-
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
103-
// also no need to report an error if the type is unresolved.
104-
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
105-
|| tcx.type_is_copy_modulo_regions(typing_env, ty)
106-
|| ty.references_error()
107-
}
108-
};
109-
if allowed && ty.needs_drop(tcx, typing_env) {
110-
// This should never happen. But we can get here e.g. in case of name resolution errors.
111-
tcx.dcx()
112-
.span_delayed_bug(span, "we should never accept maybe-dropping union or unsafe fields");
91+
// HACK (not that bad of a hack don't worry): Some codegen tests don't even define proper
92+
// impls for `Copy`. Let's short-circuit here for this validity check, since a lot of them
93+
// use unions. We should eventually fix all the tests to define that lang item or use
94+
// minicore stubs.
95+
if ty.is_trivially_pure_clone_copy() {
96+
return true;
11397
}
114-
allowed
98+
// If `BikeshedGuaranteedNoDrop` is not defined in a `#[no_core]` test, fall back to `Copy`.
99+
// This is an underapproximation of `BikeshedGuaranteedNoDrop`,
100+
let def_id = tcx
101+
.lang_items()
102+
.get(LangItem::BikeshedGuaranteedNoDrop)
103+
.unwrap_or_else(|| tcx.require_lang_item(LangItem::Copy, Some(span)));
104+
let Ok(ty) = tcx.try_normalize_erasing_regions(typing_env, ty) else {
105+
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
106+
return true;
107+
};
108+
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
109+
infcx.predicate_must_hold_modulo_regions(&Obligation::new(
110+
tcx,
111+
ObligationCause::dummy_with_span(span),
112+
param_env,
113+
ty::TraitRef::new(tcx, def_id, [ty]),
114+
))
115115
}
116116

117117
/// Check that the fields of the `union` do not need dropping.
118118
fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> bool {
119-
let item_type = tcx.type_of(item_def_id).instantiate_identity();
120-
if let ty::Adt(def, args) = item_type.kind() {
121-
assert!(def.is_union());
122-
123-
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
124-
for field in &def.non_enum_variant().fields {
125-
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
126-
else {
127-
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
128-
continue;
129-
};
119+
let def = tcx.adt_def(item_def_id);
120+
assert!(def.is_union());
130121

131-
if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
132-
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
133-
// We are currently checking the type this field came from, so it must be local.
134-
Some(Node::Field(field)) => (field.span, field.ty.span),
135-
_ => unreachable!("mir field has to correspond to hir field"),
136-
};
137-
tcx.dcx().emit_err(errors::InvalidUnionField {
138-
field_span,
139-
sugg: errors::InvalidUnionFieldSuggestion {
140-
lo: ty_span.shrink_to_lo(),
141-
hi: ty_span.shrink_to_hi(),
142-
},
143-
note: (),
144-
});
145-
return false;
146-
}
122+
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
123+
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
124+
125+
for field in &def.non_enum_variant().fields {
126+
if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
127+
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
128+
// We are currently checking the type this field came from, so it must be local.
129+
Some(Node::Field(field)) => (field.span, field.ty.span),
130+
_ => unreachable!("mir field has to correspond to hir field"),
131+
};
132+
tcx.dcx().emit_err(errors::InvalidUnionField {
133+
field_span,
134+
sugg: errors::InvalidUnionFieldSuggestion {
135+
lo: ty_span.shrink_to_lo(),
136+
hi: ty_span.shrink_to_hi(),
137+
},
138+
note: (),
139+
});
140+
return false;
147141
}
148-
} else {
149-
span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind());
150142
}
143+
151144
true
152145
}
153146

154147
/// Check that the unsafe fields do not need dropping.
155148
fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
156149
let span = tcx.def_span(item_def_id);
157-
let item_type = tcx.type_of(item_def_id).instantiate_identity();
158-
let ty::Adt(def, args) = item_type.kind() else {
159-
span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
160-
};
150+
let def = tcx.adt_def(item_def_id);
151+
161152
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
153+
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
154+
162155
for field in def.all_fields() {
163156
if !field.safety.is_unsafe() {
164157
continue;
165158
}
166-
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
167-
else {
168-
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
169-
continue;
170-
};
171159

172-
if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
160+
if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
173161
let hir::Node::Field(field) = tcx.hir_node_by_def_id(field.did.expect_local()) else {
174162
unreachable!("field has to correspond to hir field")
175163
};

compiler/rustc_middle/src/traits/select.rs

+2
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ pub enum SelectionCandidate<'tcx> {
168168
BuiltinObjectCandidate,
169169

170170
BuiltinUnsizeCandidate,
171+
172+
BikeshedGuaranteedNoDropCandidate,
171173
}
172174

173175
/// The result of trait evaluation. The order is important

compiler/rustc_middle/src/ty/adt.rs

+4
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,10 @@ impl<'tcx> rustc_type_ir::inherent::AdtDef<TyCtxt<'tcx>> for AdtDef<'tcx> {
217217
self.is_phantom_data()
218218
}
219219

220+
fn is_manually_drop(self) -> bool {
221+
self.is_manually_drop()
222+
}
223+
220224
fn all_field_tys(
221225
self,
222226
tcx: TyCtxt<'tcx>,

compiler/rustc_middle/src/ty/context.rs

+1
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ bidirectional_lang_item_map! {
690690
AsyncFnOnce,
691691
AsyncFnOnceOutput,
692692
AsyncIterator,
693+
BikeshedGuaranteedNoDrop,
693694
CallOnceFuture,
694695
CallRefFuture,
695696
Clone,

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,11 @@ where
267267
goal: Goal<I, Self>,
268268
) -> Result<Candidate<I>, NoSolution>;
269269

270+
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
271+
ecx: &mut EvalCtxt<'_, D>,
272+
goal: Goal<I, Self>,
273+
) -> Result<Candidate<I>, NoSolution>;
274+
270275
/// Consider (possibly several) candidates to upcast or unsize a type to another
271276
/// type, excluding the coercion of a sized type into a `dyn Trait`.
272277
///
@@ -478,6 +483,9 @@ where
478483
Some(TraitSolverLangItem::TransmuteTrait) => {
479484
G::consider_builtin_transmute_candidate(self, goal)
480485
}
486+
Some(TraitSolverLangItem::BikeshedGuaranteedNoDrop) => {
487+
G::consider_builtin_bikeshed_guaranteed_no_drop_candidate(self, goal)
488+
}
481489
_ => Err(NoSolution),
482490
}
483491
};

compiler/rustc_next_trait_solver/src/solve/effect_goals.rs

+7
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,13 @@ where
374374
unreachable!("TransmuteFrom is not const")
375375
}
376376

377+
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
378+
_ecx: &mut EvalCtxt<'_, D>,
379+
_goal: Goal<I, Self>,
380+
) -> Result<Candidate<I>, NoSolution> {
381+
unreachable!("BikeshedGuaranteedNoDrop is not const");
382+
}
383+
377384
fn consider_structural_builtin_unsize_candidates(
378385
_ecx: &mut EvalCtxt<'_, D>,
379386
_goal: Goal<I, Self>,

compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -942,6 +942,13 @@ where
942942
) -> Result<Candidate<I>, NoSolution> {
943943
panic!("`TransmuteFrom` does not have an associated type: {:?}", goal)
944944
}
945+
946+
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
947+
_ecx: &mut EvalCtxt<'_, D>,
948+
goal: Goal<I, Self>,
949+
) -> Result<Candidate<I>, NoSolution> {
950+
unreachable!("`BikeshedGuaranteedNoDrop` does not have an associated type: {:?}", goal)
951+
}
945952
}
946953

947954
impl<D, I> EvalCtxt<'_, D>

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

+95
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,101 @@ where
625625
})
626626
}
627627

628+
/// NOTE: This is implemented as a built-in goal and not a set of impls like:
629+
///
630+
/// ```
631+
/// impl<T> BikeshedGuaranteedNoDrop for T where T: Copy {}
632+
/// impl<T> BikeshedGuaranteedNoDrop for ManuallyDrop<T> {}
633+
/// ```
634+
///
635+
/// because these impls overlap, and I'd rather not build a coherence hack for
636+
/// this harmless overlap.
637+
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
638+
ecx: &mut EvalCtxt<'_, D>,
639+
goal: Goal<I, Self>,
640+
) -> Result<Candidate<I>, NoSolution> {
641+
if goal.predicate.polarity != ty::PredicatePolarity::Positive {
642+
return Err(NoSolution);
643+
}
644+
645+
let cx = ecx.cx();
646+
ecx.probe_builtin_trait_candidate(BuiltinImplSource::Misc).enter(|ecx| {
647+
let ty = goal.predicate.self_ty();
648+
match ty.kind() {
649+
// `&mut T` and `&T` always implement `BikeshedGuaranteedNoDrop`.
650+
ty::Ref(..) => {}
651+
// `ManuallyDrop<T>` always implements `BikeshedGuaranteedNoDrop`.
652+
ty::Adt(def, _) if def.is_manually_drop() => {}
653+
// Arrays and tuples implement `BikeshedGuaranteedNoDrop` only if
654+
// their constituent types implement `BikeshedGuaranteedNoDrop`.
655+
ty::Tuple(tys) => {
656+
ecx.add_goals(
657+
GoalSource::ImplWhereBound,
658+
tys.iter().map(|elem_ty| {
659+
goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty]))
660+
}),
661+
);
662+
}
663+
ty::Array(elem_ty, _) => {
664+
ecx.add_goal(
665+
GoalSource::ImplWhereBound,
666+
goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty])),
667+
);
668+
}
669+
670+
// All other types implement `BikeshedGuaranteedNoDrop` only if
671+
// they implement `Copy`. We could be smart here and short-circuit
672+
// some trivially `Copy`/`!Copy` types, but there's no benefit.
673+
ty::FnDef(..)
674+
| ty::FnPtr(..)
675+
| ty::Error(_)
676+
| ty::Uint(_)
677+
| ty::Int(_)
678+
| ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
679+
| ty::Bool
680+
| ty::Float(_)
681+
| ty::Char
682+
| ty::RawPtr(..)
683+
| ty::Never
684+
| ty::Pat(..)
685+
| ty::Dynamic(..)
686+
| ty::Str
687+
| ty::Slice(_)
688+
| ty::Foreign(..)
689+
| ty::Adt(..)
690+
| ty::Alias(..)
691+
| ty::Param(_)
692+
| ty::Placeholder(..)
693+
| ty::Closure(..)
694+
| ty::CoroutineClosure(..)
695+
| ty::Coroutine(..)
696+
| ty::UnsafeBinder(_)
697+
| ty::CoroutineWitness(..) => {
698+
ecx.add_goal(
699+
GoalSource::ImplWhereBound,
700+
goal.with(
701+
cx,
702+
ty::TraitRef::new(
703+
cx,
704+
cx.require_lang_item(TraitSolverLangItem::Copy),
705+
[ty],
706+
),
707+
),
708+
);
709+
}
710+
711+
ty::Bound(..)
712+
| ty::Infer(
713+
ty::TyVar(_) | ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_),
714+
) => {
715+
panic!("unexpected type `{ty:?}`")
716+
}
717+
}
718+
719+
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
720+
})
721+
}
722+
628723
/// ```ignore (builtin impl example)
629724
/// trait Trait {
630725
/// fn foo(&self);

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ symbols! {
515515
bang,
516516
begin_panic,
517517
bench,
518+
bikeshed_guaranteed_no_drop,
518519
bin,
519520
binaryheap_iter,
520521
bind_by_move_pattern_guards,

0 commit comments

Comments
 (0)