From 5cc83fd4a5fb4860f67cf0b92b41c1aa9f87a54b Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 5 Oct 2023 14:57:14 +0000 Subject: [PATCH 1/2] 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. --- compiler/rustc_interface/src/passes.rs | 8 ++++-- compiler/rustc_middle/src/thir.rs | 11 +++++++- compiler/rustc_middle/src/thir/visit.rs | 7 ++--- .../rustc_mir_build/src/build/matches/mod.rs | 4 +++ .../src/build/matches/simplify.rs | 24 ++++++++++++++--- .../rustc_mir_build/src/build/matches/test.rs | 2 ++ compiler/rustc_mir_build/src/build/mod.rs | 25 ++++++++++------- .../rustc_mir_build/src/check_unsafety.rs | 27 ++++++++++++++++--- .../src/thir/pattern/deconstruct_pat.rs | 3 ++- .../rustc_mir_build/src/thir/pattern/mod.rs | 13 ++++++--- compiler/rustc_mir_build/src/thir/print.rs | 7 +++++ .../async-unsafe-fn-call-in-safe.mir.stderr | 2 +- .../async-unsafe-fn-call-in-safe.rs | 8 ++++-- .../async-unsafe-fn-call-in-safe.thir.stderr | 18 ++++++++++++- .../const-extern-fn-requires-unsafe.rs | 1 + ...onst-extern-fn-requires-unsafe.thir.stderr | 10 ++++++- tests/ui/inline-const/pat-unsafe-err.rs | 14 ++++++++-- .../inline-const/pat-unsafe-err.thir.stderr | 19 +++++++++++++ tests/ui/inline-const/pat-unsafe.rs | 15 +++++++++-- tests/ui/inline-const/pat-unsafe.thir.stderr | 26 ++++++++++++++++++ .../non-structural-match-types.mir.stderr | 14 ++++++++++ .../ui/pattern/non-structural-match-types.rs | 3 +++ .../pattern/non-structural-match-types.stderr | 14 ---------- .../non-structural-match-types.thir.stderr | 14 ++++++++++ 24 files changed, 239 insertions(+), 50 deletions(-) create mode 100644 tests/ui/inline-const/pat-unsafe-err.thir.stderr create mode 100644 tests/ui/inline-const/pat-unsafe.thir.stderr create mode 100644 tests/ui/pattern/non-structural-match-types.mir.stderr delete mode 100644 tests/ui/pattern/non-structural-match-types.stderr create mode 100644 tests/ui/pattern/non-structural-match-types.thir.stderr diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 718dbaaafccec..461952ac4b4c9 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -775,12 +775,16 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> { rustc_hir_analysis::check_crate(tcx)?; sess.time("MIR_borrow_checking", || { - tcx.hir().par_body_owners(|def_id| tcx.ensure().mir_borrowck(def_id)); + tcx.hir().par_body_owners(|def_id| { + // Run THIR unsafety check because it's responsible for stealing + // and deallocating THIR when enabled. + tcx.ensure().thir_check_unsafety(def_id); + tcx.ensure().mir_borrowck(def_id) + }); }); sess.time("MIR_effect_checking", || { for def_id in tcx.hir().body_owners() { - tcx.ensure().thir_check_unsafety(def_id); if !tcx.sess.opts.unstable_opts.thir_unsafeck { rustc_mir_transform::check_unsafety::check_unsafety(tcx, def_id); } diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 67804998a3293..80d11c8b8f897 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -636,7 +636,8 @@ impl<'tcx> Pat<'tcx> { Wild | Range(..) | Binding { subpattern: None, .. } | Constant { .. } | Error(_) => {} AscribeUserType { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } - | Deref { subpattern } => subpattern.walk_(it), + | Deref { subpattern } + | InlineConstant { subpattern, .. } => subpattern.walk_(it), Leaf { subpatterns } | Variant { subpatterns, .. } => { subpatterns.iter().for_each(|field| field.pattern.walk_(it)) } @@ -764,6 +765,11 @@ pub enum PatKind<'tcx> { value: mir::Const<'tcx>, }, + InlineConstant { + value: mir::UnevaluatedConst<'tcx>, + subpattern: Box>, + }, + Range(Box>), /// Matches against a slice, checking the length and extracting elements. @@ -924,6 +930,9 @@ impl<'tcx> fmt::Display for Pat<'tcx> { write!(f, "{subpattern}") } PatKind::Constant { value } => write!(f, "{value}"), + PatKind::InlineConstant { value: _, ref subpattern } => { + write!(f, "{} (from inline const)", subpattern) + } PatKind::Range(box PatRange { lo, hi, end }) => { write!(f, "{lo}")?; write!(f, "{end}")?; diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index afb58438519cc..3da484e6f1b35 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -233,16 +233,17 @@ pub fn walk_pat<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, pat: &Pat<' } } Constant { value: _ } => {} + InlineConstant { value: _, subpattern } => visitor.visit_pat(subpattern), Range(_) => {} Slice { prefix, slice, suffix } | Array { prefix, slice, suffix } => { for subpattern in prefix.iter() { - visitor.visit_pat(&subpattern); + visitor.visit_pat(subpattern); } if let Some(pat) = slice { - visitor.visit_pat(&pat); + visitor.visit_pat(pat); } for subpattern in suffix.iter() { - visitor.visit_pat(&subpattern); + visitor.visit_pat(subpattern); } } Or { pats } => { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 24c6e0eae3674..1cf8c202ea4be 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -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); diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index f340feb40d4a7..e55daa6ee5f14 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -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(()); diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 30ce37a7ac1dd..5e7db7413df4f 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -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(_) => { diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index d9098bac1c2b2..3ef173c63e719 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -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()) } } }; diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 2d221b826c9bf..f0b01e94587da 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -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; diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index bbc0aeb66cfda..06b7557c03af4 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -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; diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 76ed6d2b6d73d..0d066d9800b25 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -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), diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index c957611b975c7..519622d450a8b 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -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); } diff --git a/tests/ui/async-await/async-unsafe-fn-call-in-safe.mir.stderr b/tests/ui/async-await/async-unsafe-fn-call-in-safe.mir.stderr index 2114fb59ba3a6..f9e5bf675cbd9 100644 --- a/tests/ui/async-await/async-unsafe-fn-call-in-safe.mir.stderr +++ b/tests/ui/async-await/async-unsafe-fn-call-in-safe.mir.stderr @@ -23,7 +23,7 @@ LL | S::f(); = note: consult the function's documentation for information on how to avoid undefined behavior error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/async-unsafe-fn-call-in-safe.rs:24:5 + --> $DIR/async-unsafe-fn-call-in-safe.rs:26:5 | LL | f(); | ^^^ call to unsafe function diff --git a/tests/ui/async-await/async-unsafe-fn-call-in-safe.rs b/tests/ui/async-await/async-unsafe-fn-call-in-safe.rs index c941dc27aa307..14cc0dc614fc3 100644 --- a/tests/ui/async-await/async-unsafe-fn-call-in-safe.rs +++ b/tests/ui/async-await/async-unsafe-fn-call-in-safe.rs @@ -20,6 +20,10 @@ async fn g() { } fn main() { - S::f(); //[mir]~ ERROR call to unsafe function is unsafe - f(); //[mir]~ ERROR call to unsafe function is unsafe + S::f(); + //[mir]~^ ERROR call to unsafe function is unsafe + //[thir]~^^ ERROR call to unsafe function `S::f` is unsafe + f(); + //[mir]~^ ERROR call to unsafe function is unsafe + //[thir]~^^ ERROR call to unsafe function `f` is unsafe } diff --git a/tests/ui/async-await/async-unsafe-fn-call-in-safe.thir.stderr b/tests/ui/async-await/async-unsafe-fn-call-in-safe.thir.stderr index 68d97d3fd7d5f..ba3303fe7939e 100644 --- a/tests/ui/async-await/async-unsafe-fn-call-in-safe.thir.stderr +++ b/tests/ui/async-await/async-unsafe-fn-call-in-safe.thir.stderr @@ -14,6 +14,22 @@ LL | f(); | = note: consult the function's documentation for information on how to avoid undefined behavior -error: aborting due to 2 previous errors +error[E0133]: call to unsafe function `S::f` is unsafe and requires unsafe function or block + --> $DIR/async-unsafe-fn-call-in-safe.rs:23:5 + | +LL | S::f(); + | ^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error[E0133]: call to unsafe function `f` is unsafe and requires unsafe function or block + --> $DIR/async-unsafe-fn-call-in-safe.rs:26:5 + | +LL | f(); + | ^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.rs b/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.rs index afe645ae8815c..6c4f0a5accf99 100644 --- a/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.rs +++ b/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.rs @@ -11,4 +11,5 @@ fn main() { //[thir]~^^ call to unsafe function `foo` is unsafe and requires unsafe function or block foo(); //[mir]~^ ERROR call to unsafe function is unsafe and requires unsafe function or block + //[thir]~^^ ERROR call to unsafe function `foo` is unsafe and requires unsafe function or block } diff --git a/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.thir.stderr b/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.thir.stderr index b313f06539ff7..e6b8173eb051a 100644 --- a/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.thir.stderr +++ b/tests/ui/consts/const-extern-fn/const-extern-fn-requires-unsafe.thir.stderr @@ -1,3 +1,11 @@ +error[E0133]: call to unsafe function `foo` is unsafe and requires unsafe function or block + --> $DIR/const-extern-fn-requires-unsafe.rs:12:5 + | +LL | foo(); + | ^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + error[E0133]: call to unsafe function `foo` is unsafe and requires unsafe function or block --> $DIR/const-extern-fn-requires-unsafe.rs:9:17 | @@ -6,6 +14,6 @@ LL | let a: [u8; foo()]; | = note: consult the function's documentation for information on how to avoid undefined behavior -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/inline-const/pat-unsafe-err.rs b/tests/ui/inline-const/pat-unsafe-err.rs index e290b438c5140..6df281c6d945e 100644 --- a/tests/ui/inline-const/pat-unsafe-err.rs +++ b/tests/ui/inline-const/pat-unsafe-err.rs @@ -1,11 +1,13 @@ -// ignore-test This is currently broken // revisions: mir thir +// [mir]ignore-test This is currently broken // [thir]compile-flags: -Z thir-unsafeck #![allow(incomplete_features)] #![feature(inline_const_pat)] -const unsafe fn require_unsafe() -> usize { 1 } +const unsafe fn require_unsafe() -> usize { + 1 +} fn main() { match () { @@ -14,4 +16,12 @@ fn main() { //~^ ERROR [E0133] } => (), } + + match 1 { + const { + require_unsafe() + //~^ ERROR [E0133] + }..=4 => (), + _ => (), + } } diff --git a/tests/ui/inline-const/pat-unsafe-err.thir.stderr b/tests/ui/inline-const/pat-unsafe-err.thir.stderr new file mode 100644 index 0000000000000..48a2cb4c704d5 --- /dev/null +++ b/tests/ui/inline-const/pat-unsafe-err.thir.stderr @@ -0,0 +1,19 @@ +error[E0133]: call to unsafe function `require_unsafe` is unsafe and requires unsafe function or block + --> $DIR/pat-unsafe-err.rs:15:13 + | +LL | require_unsafe(); + | ^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error[E0133]: call to unsafe function `require_unsafe` is unsafe and requires unsafe function or block + --> $DIR/pat-unsafe-err.rs:22:13 + | +LL | require_unsafe() + | ^^^^^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0133`. diff --git a/tests/ui/inline-const/pat-unsafe.rs b/tests/ui/inline-const/pat-unsafe.rs index bcf7f6e01804e..36f8632af6738 100644 --- a/tests/ui/inline-const/pat-unsafe.rs +++ b/tests/ui/inline-const/pat-unsafe.rs @@ -1,13 +1,15 @@ -// ignore-test This is currently broken // check-pass // revisions: mir thir +// [mir]ignore-test This is currently broken // [thir]compile-flags: -Z thir-unsafeck #![allow(incomplete_features)] #![warn(unused_unsafe)] #![feature(inline_const_pat)] -const unsafe fn require_unsafe() -> usize { 1 } +const unsafe fn require_unsafe() -> usize { + 1 +} fn main() { unsafe { @@ -18,5 +20,14 @@ fn main() { //~^ WARNING unnecessary `unsafe` block } => (), } + + match 1 { + const { + unsafe {} + //~^ WARNING unnecessary `unsafe` block + require_unsafe() + }..=4 => (), + _ => (), + } } } diff --git a/tests/ui/inline-const/pat-unsafe.thir.stderr b/tests/ui/inline-const/pat-unsafe.thir.stderr new file mode 100644 index 0000000000000..d62c87fc8f3c3 --- /dev/null +++ b/tests/ui/inline-const/pat-unsafe.thir.stderr @@ -0,0 +1,26 @@ +warning: unnecessary `unsafe` block + --> $DIR/pat-unsafe.rs:19:17 + | +LL | unsafe { + | ------ because it's nested under this `unsafe` block +... +LL | unsafe {} + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/pat-unsafe.rs:7:9 + | +LL | #![warn(unused_unsafe)] + | ^^^^^^^^^^^^^ + +warning: unnecessary `unsafe` block + --> $DIR/pat-unsafe.rs:26:17 + | +LL | unsafe { + | ------ because it's nested under this `unsafe` block +... +LL | unsafe {} + | ^^^^^^ unnecessary `unsafe` block + +warning: 2 warnings emitted + diff --git a/tests/ui/pattern/non-structural-match-types.mir.stderr b/tests/ui/pattern/non-structural-match-types.mir.stderr new file mode 100644 index 0000000000000..7a9e5b7e02ea0 --- /dev/null +++ b/tests/ui/pattern/non-structural-match-types.mir.stderr @@ -0,0 +1,14 @@ +error: `{closure@$DIR/non-structural-match-types.rs:12:17: 12:19}` cannot be used in patterns + --> $DIR/non-structural-match-types.rs:12:9 + | +LL | const { || {} } => {} + | ^^^^^^^^^^^^^^^ + +error: `{async block@$DIR/non-structural-match-types.rs:15:17: 15:25}` cannot be used in patterns + --> $DIR/non-structural-match-types.rs:15:9 + | +LL | const { async {} } => {} + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/pattern/non-structural-match-types.rs b/tests/ui/pattern/non-structural-match-types.rs index fc52ee3d013c1..fb7779fa80881 100644 --- a/tests/ui/pattern/non-structural-match-types.rs +++ b/tests/ui/pattern/non-structural-match-types.rs @@ -1,4 +1,7 @@ // edition:2021 +// revisions: mir thir +// [thir]compile-flags: -Z thir-unsafeck + #![allow(incomplete_features)] #![allow(unreachable_code)] #![feature(const_async_blocks)] diff --git a/tests/ui/pattern/non-structural-match-types.stderr b/tests/ui/pattern/non-structural-match-types.stderr deleted file mode 100644 index f3e0665fef51d..0000000000000 --- a/tests/ui/pattern/non-structural-match-types.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: `{closure@$DIR/non-structural-match-types.rs:9:17: 9:19}` cannot be used in patterns - --> $DIR/non-structural-match-types.rs:9:9 - | -LL | const { || {} } => {} - | ^^^^^^^^^^^^^^^ - -error: `{async block@$DIR/non-structural-match-types.rs:12:17: 12:25}` cannot be used in patterns - --> $DIR/non-structural-match-types.rs:12:9 - | -LL | const { async {} } => {} - | ^^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors - diff --git a/tests/ui/pattern/non-structural-match-types.thir.stderr b/tests/ui/pattern/non-structural-match-types.thir.stderr new file mode 100644 index 0000000000000..7a9e5b7e02ea0 --- /dev/null +++ b/tests/ui/pattern/non-structural-match-types.thir.stderr @@ -0,0 +1,14 @@ +error: `{closure@$DIR/non-structural-match-types.rs:12:17: 12:19}` cannot be used in patterns + --> $DIR/non-structural-match-types.rs:12:9 + | +LL | const { || {} } => {} + | ^^^^^^^^^^^^^^^ + +error: `{async block@$DIR/non-structural-match-types.rs:15:17: 15:25}` cannot be used in patterns + --> $DIR/non-structural-match-types.rs:15:9 + | +LL | const { async {} } => {} + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From 8aea0e959088c0a879b0cd1682ec9db6c9914ee4 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 9 Oct 2023 16:31:44 +0000 Subject: [PATCH 2/2] Address review comments Clean up code and add comments. Use InlineConstant to wrap range patterns. --- compiler/rustc_middle/src/thir.rs | 15 +++++- compiler/rustc_middle/src/thir/visit.rs | 2 +- .../src/build/matches/simplify.rs | 18 ++----- compiler/rustc_mir_build/src/build/mod.rs | 1 - .../rustc_mir_build/src/check_unsafety.rs | 27 +++-------- .../rustc_mir_build/src/thir/pattern/mod.rs | 47 +++++++++++-------- compiler/rustc_mir_build/src/thir/print.rs | 8 ++-- 7 files changed, 56 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 80d11c8b8f897..1c51c41d5a4d0 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -765,8 +765,19 @@ pub enum PatKind<'tcx> { value: mir::Const<'tcx>, }, + /// Inline constant found while lowering a pattern. InlineConstant { - value: mir::UnevaluatedConst<'tcx>, + /// [LocalDefId] of the constant, we need this so that we have a + /// reference that can be used by unsafety checking to visit nested + /// unevaluated constants. + def: LocalDefId, + /// If the inline constant is used in a range pattern, this subpattern + /// represents the range (if both ends are inline constants, there will + /// be multiple InlineConstant wrappers). + /// + /// Otherwise, the actual pattern that the constant lowered to. As with + /// other constants, inline constants are matched structurally where + /// possible. subpattern: Box>, }, @@ -930,7 +941,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { write!(f, "{subpattern}") } PatKind::Constant { value } => write!(f, "{value}"), - PatKind::InlineConstant { value: _, ref subpattern } => { + PatKind::InlineConstant { def: _, ref subpattern } => { write!(f, "{} (from inline const)", subpattern) } PatKind::Range(box PatRange { lo, hi, end }) => { diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index 3da484e6f1b35..d03d92c3a4b83 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -233,7 +233,7 @@ pub fn walk_pat<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, pat: &Pat<' } } Constant { value: _ } => {} - InlineConstant { value: _, subpattern } => visitor.visit_pat(subpattern), + InlineConstant { def: _, subpattern } => visitor.visit_pat(subpattern), Range(_) => {} Slice { prefix, slice, suffix } | Array { prefix, slice, suffix } => { for subpattern in prefix.iter() { diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index e55daa6ee5f14..32573b4d53afd 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -204,7 +204,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Err(match_pair) } - PatKind::InlineConstant { subpattern: ref pattern, value: _ } => { + PatKind::InlineConstant { subpattern: ref pattern, def: _ } => { candidate.match_pairs.push(MatchPair::new(match_pair.place, pattern, self)); Ok(()) @@ -236,20 +236,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // pattern/_match.rs for another pertinent example of this pattern). // // 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; + // `try_to_bits` if necessary. + let lo = lo.try_to_bits(sz).unwrap() ^ bias; if lo <= min { - let hi = hi - .try_eval_scalar_int(self.tcx, self.param_env) - .unwrap() - .to_bits(sz) - .unwrap() - ^ bias; + let hi = hi.try_to_bits(sz).unwrap() ^ bias; if hi > max || hi == max && end == RangeEnd::Included { // Irrefutable pattern match. return Ok(()); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 3ef173c63e719..bd96ab453b80a 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -67,7 +67,6 @@ fn mir_build<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> { thir::BodyTy::Const(ty) => construct_const(tcx, def, thir, expr, ty), }; - tcx.ensure().check_match(def); // this must run before MIR dump, because // "not all control paths return a value" is reported here. // diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index f0b01e94587da..3e0af6c56ab6e 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -3,7 +3,7 @@ use crate::errors::*; use rustc_middle::thir::visit::{self, Visitor}; use rustc_hir as hir; -use rustc_middle::mir::{BorrowKind, Const}; +use rustc_middle::mir::BorrowKind; 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 _ = self.tcx.ensure_with_value().mir_built(def); + // Runs all other queries that depend on THIR. + 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 }; @@ -279,23 +280,8 @@ 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); + PatKind::InlineConstant { def, .. } => { + self.visit_inner_body(*def); } _ => { visit::walk_pat(self, pat); @@ -808,7 +794,8 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) { } let Ok((thir, expr)) = tcx.thir_body(def) else { return }; - let _ = tcx.ensure_with_value().mir_built(def); + // Runs all other queries that depend on THIR. + 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() { diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 0d066d9800b25..dd71ab1f8e557 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -27,6 +27,7 @@ use rustc_middle::ty::{ self, AdtDef, CanonicalUserTypeAnnotation, GenericArg, GenericArgsRef, Region, Ty, TyCtxt, TypeVisitableExt, UserType, }; +use rustc_span::def_id::LocalDefId; use rustc_span::{ErrorGuaranteed, Span, Symbol}; use rustc_target::abi::{FieldIdx, Integer}; @@ -88,19 +89,21 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { fn lower_pattern_range_endpoint( &mut self, expr: Option<&'tcx hir::Expr<'tcx>>, - ) -> Result<(Option>, Option>), ErrorGuaranteed> { + ) -> Result< + (Option>, Option>, Option), + ErrorGuaranteed, + > { match expr { - None => Ok((None, None)), + None => Ok((None, 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, - ), + let (kind, ascr, inline_const) = match self.lower_lit(expr) { + PatKind::InlineConstant { subpattern, def } => { + (subpattern.kind, None, Some(def)) + } PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => { - (kind, Some(ascription)) + (kind, Some(ascription), None) } - kind => (kind, None), + kind => (kind, None, None), }; let value = if let PatKind::Constant { value } = kind { value @@ -110,7 +113,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { ); return Err(self.tcx.sess.delay_span_bug(expr.span, msg)); }; - Ok((Some(value), ascr)) + Ok((Some(value), ascr, inline_const)) } } } @@ -181,8 +184,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { return Err(self.tcx.sess.delay_span_bug(span, msg)); } - let (lo, lo_ascr) = self.lower_pattern_range_endpoint(lo_expr)?; - let (hi, hi_ascr) = self.lower_pattern_range_endpoint(hi_expr)?; + let (lo, lo_ascr, lo_inline) = self.lower_pattern_range_endpoint(lo_expr)?; + let (hi, hi_ascr, hi_inline) = self.lower_pattern_range_endpoint(hi_expr)?; let lo = lo.unwrap_or_else(|| { // Unwrap is ok because the type is known to be numeric. @@ -241,6 +244,12 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { }; } } + for inline_const in [lo_inline, hi_inline] { + if let Some(def) = inline_const { + kind = + PatKind::InlineConstant { def, subpattern: Box::new(Pat { span, ty, kind }) }; + } + } Ok(kind) } @@ -603,11 +612,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { // const eval path below. // FIXME: investigate the performance impact of removing this. let lit_input = match expr.kind { - hir::ExprKind::Lit(ref lit) => Some(LitToConstInput { lit: &lit.node, ty, neg: false }), - hir::ExprKind::Unary(hir::UnOp::Neg, ref expr) => match expr.kind { - hir::ExprKind::Lit(ref lit) => { - Some(LitToConstInput { lit: &lit.node, ty, neg: true }) - } + hir::ExprKind::Lit(lit) => Some(LitToConstInput { lit: &lit.node, ty, neg: false }), + hir::ExprKind::Unary(hir::UnOp::Neg, expr) => match expr.kind { + hir::ExprKind::Lit(lit) => Some(LitToConstInput { lit: &lit.node, ty, neg: true }), _ => None, }, _ => None, @@ -643,7 +650,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { span, None, ); - PatKind::InlineConstant { subpattern, value: uneval } + PatKind::InlineConstant { subpattern, def: def_id } } else { // If that fails, convert it to an opaque constant pattern. match tcx.const_eval_resolve(self.param_env, uneval, Some(span)) { @@ -826,8 +833,8 @@ 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::InlineConstant { def, subpattern: ref pattern } => { + PatKind::InlineConstant { def, subpattern: pattern.fold_with(folder) } } PatKind::Range(ref range) => PatKind::Range(range.clone()), PatKind::Slice { ref prefix, ref slice, ref suffix } => PatKind::Slice { diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 519622d450a8b..c3b2309b7cdae 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -692,7 +692,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { } PatKind::Deref { subpattern } => { print_indented!(self, "Deref { ", depth_lvl + 1); - print_indented!(self, "subpattern: ", depth_lvl + 2); + print_indented!(self, "subpattern:", depth_lvl + 2); self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); } @@ -701,10 +701,10 @@ 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 } => { + PatKind::InlineConstant { def, subpattern } => { print_indented!(self, "InlineConstant {", depth_lvl + 1); - print_indented!(self, format!("value: {:?}", value), depth_lvl + 2); - print_indented!(self, "subpattern: ", depth_lvl + 2); + print_indented!(self, format!("def: {:?}", def), depth_lvl + 2); + print_indented!(self, "subpattern:", depth_lvl + 2); self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); }