Skip to content

Commit 5cc83fd

Browse files
matthewjaspermatthewjasper
committed
Fix inline const pattern unsafety checking in THIR
THIR unsafety checking was getting a cycle of function unsafety checking -> building THIR for the function -> evaluating pattern inline constants in the function -> building MIR for the inline constant -> checking unsafety of functions (so that THIR can be stolen) This is fixed by not stealing THIR when generating MIR but instead when unsafety checking. This leaves an issue with pattern inline constants not being unsafety checked because they are evaluated away when generating THIR. To fix that we now represent inline constants in THIR patterns and visit them in THIR unsafety checking.
1 parent e7bdc5f commit 5cc83fd

24 files changed

+239
-50
lines changed

‎compiler/rustc_interface/src/passes.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,12 +775,16 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
775775
rustc_hir_analysis::check_crate(tcx)?;
776776

777777
sess.time("MIR_borrow_checking", || {
778-
tcx.hir().par_body_owners(|def_id| tcx.ensure().mir_borrowck(def_id));
778+
tcx.hir().par_body_owners(|def_id| {
779+
// Run THIR unsafety check because it's responsible for stealing
780+
// and deallocating THIR when enabled.
781+
tcx.ensure().thir_check_unsafety(def_id);
782+
tcx.ensure().mir_borrowck(def_id)
783+
});
779784
});
780785

781786
sess.time("MIR_effect_checking", || {
782787
for def_id in tcx.hir().body_owners() {
783-
tcx.ensure().thir_check_unsafety(def_id);
784788
if !tcx.sess.opts.unstable_opts.thir_unsafeck {
785789
rustc_mir_transform::check_unsafety::check_unsafety(tcx, def_id);
786790
}

‎compiler/rustc_middle/src/thir.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,8 @@ impl<'tcx> Pat<'tcx> {
636636
Wild | Range(..) | Binding { subpattern: None, .. } | Constant { .. } | Error(_) => {}
637637
AscribeUserType { subpattern, .. }
638638
| Binding { subpattern: Some(subpattern), .. }
639-
| Deref { subpattern } => subpattern.walk_(it),
639+
| Deref { subpattern }
640+
| InlineConstant { subpattern, .. } => subpattern.walk_(it),
640641
Leaf { subpatterns } | Variant { subpatterns, .. } => {
641642
subpatterns.iter().for_each(|field| field.pattern.walk_(it))
642643
}
@@ -764,6 +765,11 @@ pub enum PatKind<'tcx> {
764765
value: mir::Const<'tcx>,
765766
},
766767

768+
InlineConstant {
769+
value: mir::UnevaluatedConst<'tcx>,
770+
subpattern: Box<Pat<'tcx>>,
771+
},
772+
767773
Range(Box<PatRange<'tcx>>),
768774

769775
/// Matches against a slice, checking the length and extracting elements.
@@ -924,6 +930,9 @@ impl<'tcx> fmt::Display for Pat<'tcx> {
924930
write!(f, "{subpattern}")
925931
}
926932
PatKind::Constant { value } => write!(f, "{value}"),
933+
PatKind::InlineConstant { value: _, ref subpattern } => {
934+
write!(f, "{} (from inline const)", subpattern)
935+
}
927936
PatKind::Range(box PatRange { lo, hi, end }) => {
928937
write!(f, "{lo}")?;
929938
write!(f, "{end}")?;

‎compiler/rustc_middle/src/thir/visit.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,16 +233,17 @@ pub fn walk_pat<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, pat: &Pat<'
233233
}
234234
}
235235
Constant { value: _ } => {}
236+
InlineConstant { value: _, subpattern } => visitor.visit_pat(subpattern),
236237
Range(_) => {}
237238
Slice { prefix, slice, suffix } | Array { prefix, slice, suffix } => {
238239
for subpattern in prefix.iter() {
239-
visitor.visit_pat(&subpattern);
240+
visitor.visit_pat(subpattern);
240241
}
241242
if let Some(pat) = slice {
242-
visitor.visit_pat(&pat);
243+
visitor.visit_pat(pat);
243244
}
244245
for subpattern in suffix.iter() {
245-
visitor.visit_pat(&subpattern);
246+
visitor.visit_pat(subpattern);
246247
}
247248
}
248249
Or { pats } => {

‎compiler/rustc_mir_build/src/build/matches/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
847847
self.visit_primary_bindings(subpattern, subpattern_user_ty, f)
848848
}
849849

850+
PatKind::InlineConstant { ref subpattern, .. } => {
851+
self.visit_primary_bindings(subpattern, pattern_user_ty.clone(), f)
852+
}
853+
850854
PatKind::Leaf { ref subpatterns } => {
851855
for subpattern in subpatterns {
852856
let subpattern_user_ty = pattern_user_ty.clone().leaf(subpattern.field);

‎compiler/rustc_mir_build/src/build/matches/simplify.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
204204
Err(match_pair)
205205
}
206206

207+
PatKind::InlineConstant { subpattern: ref pattern, value: _ } => {
208+
candidate.match_pairs.push(MatchPair::new(match_pair.place, pattern, self));
209+
210+
Ok(())
211+
}
212+
207213
PatKind::Range(box PatRange { lo, hi, end }) => {
208214
let (range, bias) = match *lo.ty().kind() {
209215
ty::Char => {
@@ -229,11 +235,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
229235
// correct the comparison. This is achieved by XORing with a bias (see
230236
// pattern/_match.rs for another pertinent example of this pattern).
231237
//
232-
// Also, for performance, it's important to only do the second `try_to_bits` if
233-
// necessary.
234-
let lo = lo.try_to_bits(sz).unwrap() ^ bias;
238+
// Also, for performance, it's important to only do the second
239+
// `try_eval_scalar_int` if necessary.
240+
let lo = lo
241+
.try_eval_scalar_int(self.tcx, self.param_env)
242+
.unwrap()
243+
.to_bits(sz)
244+
.unwrap()
245+
^ bias;
235246
if lo <= min {
236-
let hi = hi.try_to_bits(sz).unwrap() ^ bias;
247+
let hi = hi
248+
.try_eval_scalar_int(self.tcx, self.param_env)
249+
.unwrap()
250+
.to_bits(sz)
251+
.unwrap()
252+
^ bias;
237253
if hi > max || hi == max && end == RangeEnd::Included {
238254
// Irrefutable pattern match.
239255
return Ok(());

‎compiler/rustc_mir_build/src/build/matches/test.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7373
PatKind::Or { .. } => bug!("or-patterns should have already been handled"),
7474

7575
PatKind::AscribeUserType { .. }
76+
| PatKind::InlineConstant { .. }
7677
| PatKind::Array { .. }
7778
| PatKind::Wild
7879
| PatKind::Binding { .. }
@@ -111,6 +112,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
111112
| PatKind::Or { .. }
112113
| PatKind::Binding { .. }
113114
| PatKind::AscribeUserType { .. }
115+
| PatKind::InlineConstant { .. }
114116
| PatKind::Leaf { .. }
115117
| PatKind::Deref { .. }
116118
| PatKind::Error(_) => {

‎compiler/rustc_mir_build/src/build/mod.rs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,7 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
5353
}
5454

5555
/// Construct the MIR for a given `DefId`.
56-
fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
57-
// Ensure unsafeck and abstract const building is ran before we steal the THIR.
58-
tcx.ensure_with_value()
59-
.thir_check_unsafety(tcx.typeck_root_def_id(def.to_def_id()).expect_local());
56+
fn mir_build<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> {
6057
tcx.ensure_with_value().thir_abstract_const(def);
6158
if let Err(e) = tcx.check_match(def) {
6259
return construct_error(tcx, def, e);
@@ -65,9 +62,10 @@ fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
6562
let body = match tcx.thir_body(def) {
6663
Err(error_reported) => construct_error(tcx, def, error_reported),
6764
Ok((thir, expr)) => {
68-
// We ran all queries that depended on THIR at the beginning
69-
// of `mir_build`, so now we can steal it
70-
let thir = thir.steal();
65+
let build_mir = |thir: &Thir<'tcx>| match thir.body_type {
66+
thir::BodyTy::Fn(fn_sig) => construct_fn(tcx, def, thir, expr, fn_sig),
67+
thir::BodyTy::Const(ty) => construct_const(tcx, def, thir, expr, ty),
68+
};
7169

7270
tcx.ensure().check_match(def);
7371
// this must run before MIR dump, because
@@ -76,9 +74,16 @@ fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
7674
// maybe move the check to a MIR pass?
7775
tcx.ensure().check_liveness(def);
7876

79-
match thir.body_type {
80-
thir::BodyTy::Fn(fn_sig) => construct_fn(tcx, def, &thir, expr, fn_sig),
81-
thir::BodyTy::Const(ty) => construct_const(tcx, def, &thir, expr, ty),
77+
if tcx.sess.opts.unstable_opts.thir_unsafeck {
78+
// Don't steal here if THIR unsafeck is being used. Instead
79+
// steal in unsafeck. This is so that pattern inline constants
80+
// can be evaluated as part of building the THIR of the parent
81+
// function without a cycle.
82+
build_mir(&thir.borrow())
83+
} else {
84+
// We ran all queries that depended on THIR at the beginning
85+
// of `mir_build`, so now we can steal it
86+
build_mir(&thir.steal())
8287
}
8388
}
8489
};

‎compiler/rustc_mir_build/src/check_unsafety.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::errors::*;
33
use rustc_middle::thir::visit::{self, Visitor};
44

55
use rustc_hir as hir;
6-
use rustc_middle::mir::BorrowKind;
6+
use rustc_middle::mir::{BorrowKind, Const};
77
use rustc_middle::thir::*;
88
use rustc_middle::ty::print::with_no_trimmed_paths;
99
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
@@ -124,7 +124,8 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
124124
/// Handle closures/generators/inline-consts, which is unsafecked with their parent body.
125125
fn visit_inner_body(&mut self, def: LocalDefId) {
126126
if let Ok((inner_thir, expr)) = self.tcx.thir_body(def) {
127-
let inner_thir = &inner_thir.borrow();
127+
let _ = self.tcx.ensure_with_value().mir_built(def);
128+
let inner_thir = &inner_thir.steal();
128129
let hir_context = self.tcx.hir().local_def_id_to_hir_id(def);
129130
let mut inner_visitor = UnsafetyVisitor { thir: inner_thir, hir_context, ..*self };
130131
inner_visitor.visit_expr(&inner_thir[expr]);
@@ -224,6 +225,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
224225
PatKind::Wild |
225226
// these just wrap other patterns
226227
PatKind::Or { .. } |
228+
PatKind::InlineConstant { .. } |
227229
PatKind::AscribeUserType { .. } |
228230
PatKind::Error(_) => {}
229231
}
@@ -277,6 +279,24 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
277279
visit::walk_pat(self, pat);
278280
self.inside_adt = old_inside_adt;
279281
}
282+
PatKind::Range(range) => {
283+
if let Const::Unevaluated(c, _) = range.lo {
284+
if let hir::def::DefKind::InlineConst = self.tcx.def_kind(c.def) {
285+
let def_id = c.def.expect_local();
286+
self.visit_inner_body(def_id);
287+
}
288+
}
289+
if let Const::Unevaluated(c, _) = range.hi {
290+
if let hir::def::DefKind::InlineConst = self.tcx.def_kind(c.def) {
291+
let def_id = c.def.expect_local();
292+
self.visit_inner_body(def_id);
293+
}
294+
}
295+
}
296+
PatKind::InlineConstant { value, .. } => {
297+
let def_id = value.def.expect_local();
298+
self.visit_inner_body(def_id);
299+
}
280300
_ => {
281301
visit::walk_pat(self, pat);
282302
}
@@ -788,7 +808,8 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
788808
}
789809

790810
let Ok((thir, expr)) = tcx.thir_body(def) else { return };
791-
let thir = &thir.borrow();
811+
let _ = tcx.ensure_with_value().mir_built(def);
812+
let thir = &thir.steal();
792813
// If `thir` is empty, a type error occurred, skip this body.
793814
if thir.exprs.is_empty() {
794815
return;

‎compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,8 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
13561356
let ctor;
13571357
let fields;
13581358
match &pat.kind {
1359-
PatKind::AscribeUserType { subpattern, .. } => return mkpat(subpattern),
1359+
PatKind::AscribeUserType { subpattern, .. }
1360+
| PatKind::InlineConstant { subpattern, .. } => return mkpat(subpattern),
13601361
PatKind::Binding { subpattern: Some(subpat), .. } => return mkpat(subpat),
13611362
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => {
13621363
ctor = Wildcard;

‎compiler/rustc_mir_build/src/thir/pattern/mod.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
9393
None => Ok((None, None)),
9494
Some(expr) => {
9595
let (kind, ascr) = match self.lower_lit(expr) {
96+
PatKind::InlineConstant { subpattern, value } => (
97+
PatKind::Constant { value: Const::Unevaluated(value, subpattern.ty) },
98+
None,
99+
),
96100
PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => {
97101
(kind, Some(ascription))
98102
}
@@ -633,13 +637,13 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
633637
if let Ok(Some(valtree)) =
634638
self.tcx.const_eval_resolve_for_typeck(self.param_env, ct, Some(span))
635639
{
636-
self.const_to_pat(
640+
let subpattern = self.const_to_pat(
637641
Const::Ty(ty::Const::new_value(self.tcx, valtree, ty)),
638642
id,
639643
span,
640644
None,
641-
)
642-
.kind
645+
);
646+
PatKind::InlineConstant { subpattern, value: uneval }
643647
} else {
644648
// If that fails, convert it to an opaque constant pattern.
645649
match tcx.const_eval_resolve(self.param_env, uneval, Some(span)) {
@@ -822,6 +826,9 @@ impl<'tcx> PatternFoldable<'tcx> for PatKind<'tcx> {
822826
PatKind::Deref { subpattern: subpattern.fold_with(folder) }
823827
}
824828
PatKind::Constant { value } => PatKind::Constant { value },
829+
PatKind::InlineConstant { value, subpattern: ref pattern } => {
830+
PatKind::InlineConstant { value, subpattern: pattern.fold_with(folder) }
831+
}
825832
PatKind::Range(ref range) => PatKind::Range(range.clone()),
826833
PatKind::Slice { ref prefix, ref slice, ref suffix } => PatKind::Slice {
827834
prefix: prefix.fold_with(folder),

0 commit comments

Comments
 (0)