From a992defc8b0d7937d0422003984b7b5d3e5f472e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 5 Oct 2023 09:39:33 +0000 Subject: [PATCH 1/3] Remove mir::Const::from_anon_const --- compiler/rustc_middle/src/mir/consts.rs | 102 +------------------ compiler/rustc_middle/src/ty/consts.rs | 4 + compiler/rustc_mir_build/src/thir/cx/expr.rs | 26 ++++- 3 files changed, 29 insertions(+), 103 deletions(-) diff --git a/compiler/rustc_middle/src/mir/consts.rs b/compiler/rustc_middle/src/mir/consts.rs index 84a9ab4013b..fa7fa8e8c74 100644 --- a/compiler/rustc_middle/src/mir/consts.rs +++ b/compiler/rustc_middle/src/mir/consts.rs @@ -1,17 +1,16 @@ use std::fmt::{self, Debug, Display, Formatter}; use rustc_hir; -use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::{self as hir}; +use rustc_hir::def_id::DefId; use rustc_session::RemapFileNameExt; use rustc_span::Span; use rustc_target::abi::{HasDataLayout, Size}; use crate::mir::interpret::{alloc_range, AllocId, ConstAllocation, ErrorHandled, Scalar}; use crate::mir::{pretty_print_const_value, Promoted}; +use crate::ty::GenericArgsRef; use crate::ty::ScalarInt; -use crate::ty::{self, print::pretty_print_const, List, Ty, TyCtxt}; -use crate::ty::{GenericArgs, GenericArgsRef}; +use crate::ty::{self, print::pretty_print_const, Ty, TyCtxt}; /////////////////////////////////////////////////////////////////////////// /// Evaluated Constants @@ -399,101 +398,6 @@ impl<'tcx> Const<'tcx> { Self::Val(val, ty) } - /// Literals are converted to `Const::Val`, const generic parameters are eagerly - /// converted to a constant, everything else becomes `Unevaluated`. - #[instrument(skip(tcx), level = "debug", ret)] - pub fn from_anon_const( - tcx: TyCtxt<'tcx>, - def: LocalDefId, - param_env: ty::ParamEnv<'tcx>, - ) -> Self { - let body_id = match tcx.hir().get_by_def_id(def) { - hir::Node::AnonConst(ac) => ac.body, - _ => { - span_bug!(tcx.def_span(def), "from_anon_const can only process anonymous constants") - } - }; - - let expr = &tcx.hir().body(body_id).value; - debug!(?expr); - - // Unwrap a block, so that e.g. `{ P }` is recognised as a parameter. Const arguments - // currently have to be wrapped in curly brackets, so it's necessary to special-case. - let expr = match &expr.kind { - hir::ExprKind::Block(block, _) if block.stmts.is_empty() && block.expr.is_some() => { - block.expr.as_ref().unwrap() - } - _ => expr, - }; - debug!("expr.kind: {:?}", expr.kind); - - let ty = tcx.type_of(def).instantiate_identity(); - debug!(?ty); - - // FIXME(const_generics): We currently have to special case parameters because `min_const_generics` - // does not provide the parents generics to anonymous constants. We still allow generic const - // parameters by themselves however, e.g. `N`. These constants would cause an ICE if we were to - // ever try to substitute the generic parameters in their bodies. - // - // While this doesn't happen as these constants are always used as `ty::ConstKind::Param`, it does - // cause issues if we were to remove that special-case and try to evaluate the constant instead. - use hir::{def::DefKind::ConstParam, def::Res, ExprKind, Path, QPath}; - match expr.kind { - ExprKind::Path(QPath::Resolved(_, &Path { res: Res::Def(ConstParam, def_id), .. })) => { - // Find the name and index of the const parameter by indexing the generics of - // the parent item and construct a `ParamConst`. - let item_def_id = tcx.parent(def_id); - let generics = tcx.generics_of(item_def_id); - let index = generics.param_def_id_to_index[&def_id]; - let name = tcx.item_name(def_id); - let ty_const = ty::Const::new_param(tcx, ty::ParamConst::new(index, name), ty); - debug!(?ty_const); - - return Self::Ty(ty_const); - } - _ => {} - } - - let hir_id = tcx.hir().local_def_id_to_hir_id(def); - let parent_args = if let Some(parent_hir_id) = tcx.hir().opt_parent_id(hir_id) - && let Some(parent_did) = parent_hir_id.as_owner() - { - GenericArgs::identity_for_item(tcx, parent_did) - } else { - List::empty() - }; - debug!(?parent_args); - - let did = def.to_def_id(); - let child_args = GenericArgs::identity_for_item(tcx, did); - let args = tcx.mk_args_from_iter(parent_args.into_iter().chain(child_args.into_iter())); - debug!(?args); - - let span = tcx.def_span(def); - let uneval = UnevaluatedConst::new(did, args); - debug!(?span, ?param_env); - - match tcx.const_eval_resolve(param_env, uneval, Some(span)) { - Ok(val) => { - debug!("evaluated const value"); - Self::Val(val, ty) - } - Err(_) => { - debug!("error encountered during evaluation"); - // Error was handled in `const_eval_resolve`. Here we just create a - // new unevaluated const and error hard later in codegen - Self::Unevaluated( - UnevaluatedConst { - def: did, - args: GenericArgs::identity_for_item(tcx, did), - promoted: None, - }, - ty, - ) - } - } - } - pub fn from_ty_const(c: ty::Const<'tcx>, tcx: TyCtxt<'tcx>) -> Self { match c.kind() { ty::ConstKind::Value(valtree) => { diff --git a/compiler/rustc_middle/src/ty/consts.rs b/compiler/rustc_middle/src/ty/consts.rs index 0f5817c78e0..d0a1c943292 100644 --- a/compiler/rustc_middle/src/ty/consts.rs +++ b/compiler/rustc_middle/src/ty/consts.rs @@ -216,6 +216,10 @@ impl<'tcx> Const<'tcx> { } } + // FIXME(const_generics): We currently have to special case parameters because `min_const_generics` + // does not provide the parents generics to anonymous constants. We still allow generic const + // parameters by themselves however, e.g. `N`. These constants would cause an ICE if we were to + // ever try to substitute the generic parameters in their bodies. match expr.kind { hir::ExprKind::Path(hir::QPath::Resolved( _, diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 6541b4f6a3e..e01ee45180f 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -642,15 +642,33 @@ impl<'tcx> Cx<'tcx> { } } hir::InlineAsmOperand::Const { ref anon_const } => { - let value = - mir::Const::from_anon_const(tcx, anon_const.def_id, self.param_env); + let value = mir::Const::Unevaluated( + mir::UnevaluatedConst { + def: anon_const.def_id.to_def_id(), + args: GenericArgs::identity_for_item( + self.tcx, + anon_const.def_id, + ), + promoted: None, + }, + tcx.type_of(anon_const.def_id).instantiate_identity(), + ); let span = tcx.def_span(anon_const.def_id); InlineAsmOperand::Const { value, span } } hir::InlineAsmOperand::SymFn { ref anon_const } => { - let value = - mir::Const::from_anon_const(tcx, anon_const.def_id, self.param_env); + let value = mir::Const::Unevaluated( + mir::UnevaluatedConst { + def: anon_const.def_id.to_def_id(), + args: GenericArgs::identity_for_item( + self.tcx, + anon_const.def_id, + ), + promoted: None, + }, + tcx.type_of(anon_const.def_id).instantiate_identity(), + ); let span = tcx.def_span(anon_const.def_id); InlineAsmOperand::SymFn { value, span } From 82f23d56b758d87e5eee642946ad8a422e6542ba Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 5 Oct 2023 09:43:55 +0000 Subject: [PATCH 2/3] make sure we still eagerly emit errors --- compiler/rustc_mir_build/src/thir/cx/expr.rs | 6 ++++-- tests/ui/asm/const-error.rs | 15 +++++++++++++++ tests/ui/asm/const-error.stderr | 9 +++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 tests/ui/asm/const-error.rs create mode 100644 tests/ui/asm/const-error.stderr diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index e01ee45180f..b07ffe7293b 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -652,7 +652,8 @@ impl<'tcx> Cx<'tcx> { promoted: None, }, tcx.type_of(anon_const.def_id).instantiate_identity(), - ); + ) + .normalize(tcx, self.param_env); let span = tcx.def_span(anon_const.def_id); InlineAsmOperand::Const { value, span } @@ -668,7 +669,8 @@ impl<'tcx> Cx<'tcx> { promoted: None, }, tcx.type_of(anon_const.def_id).instantiate_identity(), - ); + ) + .normalize(tcx, self.param_env); let span = tcx.def_span(anon_const.def_id); InlineAsmOperand::SymFn { value, span } diff --git a/tests/ui/asm/const-error.rs b/tests/ui/asm/const-error.rs new file mode 100644 index 00000000000..4e14becf74b --- /dev/null +++ b/tests/ui/asm/const-error.rs @@ -0,0 +1,15 @@ +// only-x86_64 +// needs-asm-support + +#![feature(asm_const)] + +// Test to make sure that we emit const errors eagerly for inline asm + +use std::arch::asm; + +fn test() { + unsafe { asm!("/* {} */", const 1 / 0); } + //~^ ERROR evaluation of +} + +fn main() {} diff --git a/tests/ui/asm/const-error.stderr b/tests/ui/asm/const-error.stderr new file mode 100644 index 00000000000..fe311832177 --- /dev/null +++ b/tests/ui/asm/const-error.stderr @@ -0,0 +1,9 @@ +error[E0080]: evaluation of `test::::{constant#0}` failed + --> $DIR/const-error.rs:11:37 + | +LL | unsafe { asm!("/* {} */", const 1 / 0); } + | ^^^^^ attempt to divide `1_i32` by zero + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. From 8bf9c18914b8c474f15da39fa6ce3e2522d5baa5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 17 Nov 2023 23:49:07 +0000 Subject: [PATCH 3/3] Review comment --- compiler/rustc_middle/src/mir/consts.rs | 11 ++++++++ compiler/rustc_mir_build/src/thir/cx/expr.rs | 28 ++++++-------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_middle/src/mir/consts.rs b/compiler/rustc_middle/src/mir/consts.rs index fa7fa8e8c74..d38df1b7201 100644 --- a/compiler/rustc_middle/src/mir/consts.rs +++ b/compiler/rustc_middle/src/mir/consts.rs @@ -219,6 +219,17 @@ pub enum Const<'tcx> { } impl<'tcx> Const<'tcx> { + pub fn identity_unevaluated(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::EarlyBinder> { + ty::EarlyBinder::bind(Const::Unevaluated( + UnevaluatedConst { + def: def_id, + args: ty::GenericArgs::identity_for_item(tcx, def_id), + promoted: None, + }, + tcx.type_of(def_id).skip_binder(), + )) + } + #[inline(always)] pub fn ty(&self) -> Ty<'tcx> { match self { diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index b07ffe7293b..d3af1b7745e 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -642,34 +642,22 @@ impl<'tcx> Cx<'tcx> { } } hir::InlineAsmOperand::Const { ref anon_const } => { - let value = mir::Const::Unevaluated( - mir::UnevaluatedConst { - def: anon_const.def_id.to_def_id(), - args: GenericArgs::identity_for_item( - self.tcx, - anon_const.def_id, - ), - promoted: None, - }, - tcx.type_of(anon_const.def_id).instantiate_identity(), + let value = mir::Const::identity_unevaluated( + tcx, + anon_const.def_id.to_def_id(), ) + .instantiate_identity() .normalize(tcx, self.param_env); let span = tcx.def_span(anon_const.def_id); InlineAsmOperand::Const { value, span } } hir::InlineAsmOperand::SymFn { ref anon_const } => { - let value = mir::Const::Unevaluated( - mir::UnevaluatedConst { - def: anon_const.def_id.to_def_id(), - args: GenericArgs::identity_for_item( - self.tcx, - anon_const.def_id, - ), - promoted: None, - }, - tcx.type_of(anon_const.def_id).instantiate_identity(), + let value = mir::Const::identity_unevaluated( + tcx, + anon_const.def_id.to_def_id(), ) + .instantiate_identity() .normalize(tcx, self.param_env); let span = tcx.def_span(anon_const.def_id);