Skip to content

Commit 57c250f

Browse files
Implement and use BikeshedGuaranteedNoDrop for union/unsafe field validity
1 parent 415c228 commit 57c250f

File tree

23 files changed

+378
-72
lines changed

23 files changed

+378
-72
lines changed

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

+40-65
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,63 @@ 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");
113-
}
114-
allowed
91+
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
92+
infcx.predicate_must_hold_modulo_regions(&Obligation::new(
93+
tcx,
94+
ObligationCause::dummy_with_span(span),
95+
param_env,
96+
ty::TraitRef::new(
97+
tcx,
98+
tcx.require_lang_item(LangItem::BikeshedGuaranteedNoDrop, Some(span)),
99+
[ty],
100+
),
101+
))
115102
}
116103

117104
/// Check that the fields of the `union` do not need dropping.
118105
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-
};
106+
let def = tcx.adt_def(item_def_id);
107+
assert!(def.is_union());
130108

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-
}
109+
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
110+
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
111+
112+
for field in &def.non_enum_variant().fields {
113+
if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
114+
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
115+
// We are currently checking the type this field came from, so it must be local.
116+
Some(Node::Field(field)) => (field.span, field.ty.span),
117+
_ => unreachable!("mir field has to correspond to hir field"),
118+
};
119+
tcx.dcx().emit_err(errors::InvalidUnionField {
120+
field_span,
121+
sugg: errors::InvalidUnionFieldSuggestion {
122+
lo: ty_span.shrink_to_lo(),
123+
hi: ty_span.shrink_to_hi(),
124+
},
125+
note: (),
126+
});
127+
return false;
147128
}
148-
} else {
149-
span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind());
150129
}
130+
151131
true
152132
}
153133

154134
/// Check that the unsafe fields do not need dropping.
155135
fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
156136
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-
};
137+
let def = tcx.adt_def(item_def_id);
138+
161139
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
140+
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);
141+
162142
for field in def.all_fields() {
163143
if !field.safety.is_unsafe() {
164144
continue;
165145
}
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-
};
171146

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

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
@@ -682,6 +682,7 @@ bidirectional_lang_item_map! {
682682
AsyncFnOnce,
683683
AsyncFnOnceOutput,
684684
AsyncIterator,
685+
BikeshedGuaranteedNoDrop,
685686
CallOnceFuture,
686687
CallRefFuture,
687688
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
@@ -373,6 +373,13 @@ where
373373
unreachable!("TransmuteFrom is not const")
374374
}
375375

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

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

+7
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,13 @@ where
930930
) -> Result<Candidate<I>, NoSolution> {
931931
panic!("`TransmuteFrom` does not have an associated type: {:?}", goal)
932932
}
933+
934+
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
935+
_ecx: &mut EvalCtxt<'_, D>,
936+
goal: Goal<I, Self>,
937+
) -> Result<Candidate<I>, NoSolution> {
938+
unreachable!("`BikeshedGuaranteedNoDrop` does not have an associated type: {:?}", goal)
939+
}
933940
}
934941

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

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

+95
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,101 @@ where
622622
})
623623
}
624624

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

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ symbols! {
514514
bang,
515515
begin_panic,
516516
bench,
517+
bikeshed_guaranteed_no_drop,
517518
bin,
518519
binaryheap_iter,
519520
bind_by_move_pattern_guards,

0 commit comments

Comments
 (0)