From 2d8ed998042f2ecbabb10f2adfb9d311f328ebcf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 9 Oct 2023 16:31:44 +0000 Subject: [PATCH] Address review comments Clean up code and add comments. Use InlineConstant to wrap range patterns. --- compiler/rustc_middle/src/thir.rs | 10 +++++ compiler/rustc_mir_build/src/build/mod.rs | 1 - .../rustc_mir_build/src/check_unsafety.rs | 22 +++------- .../rustc_mir_build/src/thir/pattern/mod.rs | 44 +++++++++++-------- compiler/rustc_mir_build/src/thir/print.rs | 4 +- 5 files changed, 43 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 6e908ff7735f..6fb56a35bda9 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -749,8 +749,18 @@ pub enum PatKind<'tcx> { value: mir::Const<'tcx>, }, + /// Pattern lowered from an inline constant InlineConstant { + /// Unevaluated version of the constant, we need this for: + /// 1. Having a reference that can be used by unsafety checking to visit nested + /// unevaluated constants. + /// 2. During THIR building we turn this back to a [Self::Constant] in range patterns. value: mir::UnevaluatedConst<'tcx>, + /// 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>, }, diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index b6b36fb43714..31e4a6d76fce 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 dbc20cf36c22..e837e6d0bee7 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 }; @@ -278,20 +279,6 @@ 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); @@ -804,7 +791,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 41a7719c0594..1fd211e202a2 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -20,7 +20,9 @@ use rustc_index::Idx; use rustc_middle::mir::interpret::{ ErrorHandled, GlobalId, LitToConstError, LitToConstInput, Scalar, }; -use rustc_middle::mir::{self, BorrowKind, Const, Mutability, UserTypeProjection}; +use rustc_middle::mir::{ + self, BorrowKind, Const, Mutability, UnevaluatedConst, UserTypeProjection, +}; use rustc_middle::thir::{Ascription, BindingMode, FieldPat, LocalVarId, Pat, PatKind, PatRange}; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{ @@ -88,19 +90,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, value } => { + (subpattern.kind, None, Some(value)) + } 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 +114,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 +185,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 +245,12 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { }; } } + for inline_const in [lo_inline, hi_inline] { + if let Some(value) = inline_const { + kind = + PatKind::InlineConstant { value, subpattern: Box::new(Pat { span, ty, kind }) }; + } + } Ok(kind) } @@ -606,11 +616,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, diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 5e231a32d52b..34cbef8fb345 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); } @@ -704,7 +704,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { 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); + print_indented!(self, "subpattern:", depth_lvl + 2); self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); }