bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Fix inline const pattern unsafety checking in THIR

Browse source 
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.
This commit is contained in:
Matthew Jasper 2023-10-05 14:57:14 +00:00
parent e7bdc5f9f8
commit 5cc83fd4a5
24 changed files with 239 additions and 50 deletions
Download patch file Download diff file Expand all files Collapse all files

4
compiler/rustc_mir_build/src/build/matches/mod.rs
View file

@ -847,6 +847,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.visit_primary_bindings(subpattern, subpattern_user_ty, f)
}
PatKind::InlineConstant { ref subpattern, .. } => {
self.visit_primary_bindings(subpattern, pattern_user_ty.clone(), f)
}
PatKind::Leaf { ref subpatterns } => {
for subpattern in subpatterns {
let subpattern_user_ty = pattern_user_ty.clone().leaf(subpattern.field);

24
compiler/rustc_mir_build/src/build/matches/simplify.rs
View file

@ -204,6 +204,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Err(match_pair)
}
PatKind::InlineConstant { subpattern: ref pattern, value: _ } => {
candidate.match_pairs.push(MatchPair::new(match_pair.place, pattern, self));
Ok(())
}
PatKind::Range(box PatRange { lo, hi, end }) => {
let (range, bias) = match *lo.ty().kind() {
ty::Char => {
@ -229,11 +235,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// correct the comparison. This is achieved by XORing with a bias (see
// pattern/_match.rs for another pertinent example of this pattern).
//
// Also, for performance, it's important to only do the second `try_to_bits` if
// necessary.
let lo = lo.try_to_bits(sz).unwrap() ^ bias;
// Also, for performance, it's important to only do the second
// `try_eval_scalar_int` if necessary.
let lo = lo
.try_eval_scalar_int(self.tcx, self.param_env)
.unwrap()
.to_bits(sz)
.unwrap()
^ bias;
if lo <= min {
let hi = hi.try_to_bits(sz).unwrap() ^ bias;
let hi = hi
.try_eval_scalar_int(self.tcx, self.param_env)
.unwrap()
.to_bits(sz)
.unwrap()
^ bias;
if hi > max || hi == max && end == RangeEnd::Included {
// Irrefutable pattern match.
return Ok(());

2
compiler/rustc_mir_build/src/build/matches/test.rs
View file

@ -73,6 +73,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
PatKind::Or { .. } => bug!("or-patterns should have already been handled"),
PatKind::AscribeUserType { .. }
| PatKind::InlineConstant { .. }
| PatKind::Array { .. }
| PatKind::Wild
| PatKind::Binding { .. }
@ -111,6 +112,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| PatKind::Or { .. }
| PatKind::Binding { .. }
| PatKind::AscribeUserType { .. }
| PatKind::InlineConstant { .. }
| PatKind::Leaf { .. }
| PatKind::Deref { .. }
| PatKind::Error(_) => {

25
compiler/rustc_mir_build/src/build/mod.rs
View file

@ -53,10 +53,7 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
}
/// Construct the MIR for a given `DefId`.
fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
// Ensure unsafeck and abstract const building is ran before we steal the THIR.
tcx.ensure_with_value()
.thir_check_unsafety(tcx.typeck_root_def_id(def.to_def_id()).expect_local());
fn mir_build<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> {
tcx.ensure_with_value().thir_abstract_const(def);
if let Err(e) = tcx.check_match(def) {
return construct_error(tcx, def, e);
@ -65,9 +62,10 @@ fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
let body = match tcx.thir_body(def) {
Err(error_reported) => construct_error(tcx, def, error_reported),
Ok((thir, expr)) => {
// We ran all queries that depended on THIR at the beginning
// of `mir_build`, so now we can steal it
let thir = thir.steal();
let build_mir = |thir: &Thir<'tcx>| match thir.body_type {
thir::BodyTy::Fn(fn_sig) => construct_fn(tcx, def, thir, expr, fn_sig),
thir::BodyTy::Const(ty) => construct_const(tcx, def, thir, expr, ty),
};
tcx.ensure().check_match(def);
// this must run before MIR dump, because
@ -76,9 +74,16 @@ fn mir_build(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
// maybe move the check to a MIR pass?
tcx.ensure().check_liveness(def);
match thir.body_type {
thir::BodyTy::Fn(fn_sig) => construct_fn(tcx, def, &thir, expr, fn_sig),
thir::BodyTy::Const(ty) => construct_const(tcx, def, &thir, expr, ty),
if tcx.sess.opts.unstable_opts.thir_unsafeck {
// Don't steal here if THIR unsafeck is being used. Instead
// steal in unsafeck. This is so that pattern inline constants
// can be evaluated as part of building the THIR of the parent
// function without a cycle.
build_mir(&thir.borrow())
} else {
// We ran all queries that depended on THIR at the beginning
// of `mir_build`, so now we can steal it
build_mir(&thir.steal())
}
}
};

27
compiler/rustc_mir_build/src/check_unsafety.rs
View file

@ -3,7 +3,7 @@ use crate::errors::*;
use rustc_middle::thir::visit::{self, Visitor};
use rustc_hir as hir;
use rustc_middle::mir::BorrowKind;
use rustc_middle::mir::{BorrowKind, Const};
use rustc_middle::thir::*;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
@ -124,7 +124,8 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
/// Handle closures/generators/inline-consts, which is unsafecked with their parent body.
fn visit_inner_body(&mut self, def: LocalDefId) {
if let Ok((inner_thir, expr)) = self.tcx.thir_body(def) {
let inner_thir = &inner_thir.borrow();
let _ = self.tcx.ensure_with_value().mir_built(def);
let inner_thir = &inner_thir.steal();
let hir_context = self.tcx.hir().local_def_id_to_hir_id(def);
let mut inner_visitor = UnsafetyVisitor { thir: inner_thir, hir_context, ..*self };
inner_visitor.visit_expr(&inner_thir[expr]);
@ -224,6 +225,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
PatKind::Wild |
// these just wrap other patterns
PatKind::Or { .. } |
PatKind::InlineConstant { .. } |
PatKind::AscribeUserType { .. } |
PatKind::Error(_) => {}
}
@ -277,6 +279,24 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
visit::walk_pat(self, pat);
self.inside_adt = old_inside_adt;
}
PatKind::Range(range) => {
if let Const::Unevaluated(c, _) = range.lo {
if let hir::def::DefKind::InlineConst = self.tcx.def_kind(c.def) {
let def_id = c.def.expect_local();
self.visit_inner_body(def_id);
}
}
if let Const::Unevaluated(c, _) = range.hi {
if let hir::def::DefKind::InlineConst = self.tcx.def_kind(c.def) {
let def_id = c.def.expect_local();
self.visit_inner_body(def_id);
}
}
}
PatKind::InlineConstant { value, .. } => {
let def_id = value.def.expect_local();
self.visit_inner_body(def_id);
}
_ => {
visit::walk_pat(self, pat);
}
@ -788,7 +808,8 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
}
let Ok((thir, expr)) = tcx.thir_body(def) else { return };
let thir = &thir.borrow();
let _ = tcx.ensure_with_value().mir_built(def);
let thir = &thir.steal();
// If `thir` is empty, a type error occurred, skip this body.
if thir.exprs.is_empty() {
return;

3
compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
View file

@ -1356,7 +1356,8 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
let ctor;
let fields;
match &pat.kind {
PatKind::AscribeUserType { subpattern, .. } => return mkpat(subpattern),
PatKind::AscribeUserType { subpattern, .. }
| PatKind::InlineConstant { subpattern, .. } => return mkpat(subpattern),
PatKind::Binding { subpattern: Some(subpat), .. } => return mkpat(subpat),
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => {
ctor = Wildcard;

13
compiler/rustc_mir_build/src/thir/pattern/mod.rs
View file

@ -93,6 +93,10 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
None => Ok((None, None)),
Some(expr) => {
let (kind, ascr) = match self.lower_lit(expr) {
PatKind::InlineConstant { subpattern, value } => (
PatKind::Constant { value: Const::Unevaluated(value, subpattern.ty) },
None,
),
PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => {
(kind, Some(ascription))
}
@ -633,13 +637,13 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
if let Ok(Some(valtree)) =
self.tcx.const_eval_resolve_for_typeck(self.param_env, ct, Some(span))
{
self.const_to_pat(
let subpattern = self.const_to_pat(
Const::Ty(ty::Const::new_value(self.tcx, valtree, ty)),
id,
span,
None,
)
.kind
);
PatKind::InlineConstant { subpattern, value: uneval }
} else {
// If that fails, convert it to an opaque constant pattern.
match tcx.const_eval_resolve(self.param_env, uneval, Some(span)) {
@ -822,6 +826,9 @@ impl<'tcx> PatternFoldable<'tcx> for PatKind<'tcx> {
PatKind::Deref { subpattern: subpattern.fold_with(folder) }
}
PatKind::Constant { value } => PatKind::Constant { value },
PatKind::InlineConstant { value, subpattern: ref pattern } => {
PatKind::InlineConstant { value, subpattern: pattern.fold_with(folder) }
}
PatKind::Range(ref range) => PatKind::Range(range.clone()),
PatKind::Slice { ref prefix, ref slice, ref suffix } => PatKind::Slice {
prefix: prefix.fold_with(folder),

7
compiler/rustc_mir_build/src/thir/print.rs
View file

@ -701,6 +701,13 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
print_indented!(self, format!("value: {:?}", value), depth_lvl + 2);
print_indented!(self, "}", depth_lvl + 1);
}
PatKind::InlineConstant { value, subpattern } => {
print_indented!(self, "InlineConstant {", depth_lvl + 1);
print_indented!(self, format!("value: {:?}", value), depth_lvl + 2);
print_indented!(self, "subpattern: ", depth_lvl + 2);
self.print_pat(subpattern, depth_lvl + 2);
print_indented!(self, "}", depth_lvl + 1);
}
PatKind::Range(pat_range) => {
print_indented!(self, format!("Range ( {:?} )", pat_range), depth_lvl + 1);
}