Auto merge of #116482 - matthewjasper:thir-unsafeck-inline-constants, r=b-naber

Fix inline const pattern unsafety checking in THIR

Fix THIR unsafety checking of inline constants.
- Steal THIR in THIR unsafety checking (if enabled) instead of MIR lowering.
- Represent inline constants in THIR patterns.

Author: bors

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);
             }

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,22 @@ pub enum PatKind<'tcx> {
         value: mir::Const<'tcx>,
     },
 
+    /// Inline constant found while lowering a pattern.
+    InlineConstant {
+        /// [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<Pat<'tcx>>,
+    },
+
     Range(Box<PatRange<'tcx>>),
 
     /// Matches against a slice, checking the length and extracting elements.
@@ -924,6 +941,9 @@ impl<'tcx> fmt::Display for Pat<'tcx> {
                 write!(f, "{subpattern}")
             }
             PatKind::Constant { value } => write!(f, "{value}"),
+            PatKind::InlineConstant { def: _, ref subpattern } => {
+                write!(f, "{} (from inline const)", subpattern)
+            }
             PatKind::Range(box PatRange { lo, hi, end }) => {
                 write!(f, "{lo}")?;
                 write!(f, "{end}")?;

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 { def: _, 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 } => {

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);

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, def: _ } => {
+                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,8 +235,8 @@ 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.
+                    // 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;
                     if lo <= min {
                         let hi = hi.try_to_bits(sz).unwrap() ^ bias;

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(_) => {

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,20 +62,27 @@ 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
             // "not all control paths return a value" is reported here.
             //
             // 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())
             }
         }
     };

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -124,7 +124,9 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
     /// Handle closures/coroutines/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();
+            // 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 };
             inner_visitor.visit_expr(&inner_thir[expr]);
@@ -224,6 +226,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 +280,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                 visit::walk_pat(self, pat);
                 self.inside_adt = old_inside_adt;
             }
+            PatKind::InlineConstant { def, .. } => {
+                self.visit_inner_body(*def);
+            }
             _ => {
                 visit::walk_pat(self, pat);
             }
@@ -788,7 +794,9 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
     }
 
     let Ok((thir, expr)) = tcx.thir_body(def) else { return };
-    let thir = &thir.borrow();
+    // 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() {
         return;

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

@@ -1326,7 +1326,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;

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,15 +89,21 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
     fn lower_pattern_range_endpoint(
         &mut self,
         expr: Option<&'tcx hir::Expr<'tcx>>,
-    ) -> Result<(Option<mir::Const<'tcx>>, Option<Ascription<'tcx>>), ErrorGuaranteed> {
+    ) -> Result<
+        (Option<mir::Const<'tcx>>, Option<Ascription<'tcx>>, Option<LocalDefId>),
+        ErrorGuaranteed,
+    > {
         match expr {
-            None => Ok((None, None)),
+            None => Ok((None, None, None)),
             Some(expr) => {
-                let (kind, ascr) = match self.lower_lit(expr) {
+                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
@@ -106,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))
             }
         }
     }
@@ -177,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.
@@ -237,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)
     }
 
@@ -599,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,
@@ -633,13 +644,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, 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)) {
@@ -822,6 +833,9 @@ impl<'tcx> PatternFoldable<'tcx> for PatKind<'tcx> {
                 PatKind::Deref { subpattern: subpattern.fold_with(folder) }
             }
             PatKind::Constant { value } => PatKind::Constant { value },
+            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 {
                 prefix: prefix.fold_with(folder),

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,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 { def, subpattern } => {
+                print_indented!(self, "InlineConstant {", depth_lvl + 1);
+                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);
+            }
             PatKind::Range(pat_range) => {
                 print_indented!(self, format!("Range ( {:?} )", pat_range), depth_lvl + 1);
             }

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

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
 }