From 168de14ac9f3ec0b0f6fa7f5328336e6ce97d60b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 16:02:11 +0000 Subject: [PATCH] Make simd_shuffle_indices use valtrees --- compiler/rustc_codegen_ssa/src/mir/block.rs | 11 +++- .../rustc_codegen_ssa/src/mir/constant.rs | 23 ++++--- .../rustc_const_eval/src/const_eval/mod.rs | 1 - .../rustc_middle/src/mir/interpret/queries.rs | 26 +++----- .../clippy/clippy_lints/src/non_copy_const.rs | 64 +++++++++++++++---- .../clippy/tests/ui/crashes/ice-9445.stderr | 12 ++++ 6 files changed, 92 insertions(+), 45 deletions(-) create mode 100644 src/tools/clippy/tests/ui/crashes/ice-9445.stderr diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 0cec560ba4518..d53bffb3e03cc 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -863,7 +863,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // promotes any complex rvalues to constants. if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { if let mir::Operand::Constant(constant) = arg { - let c = self.eval_mir_constant(constant); + let ct = self.monomorphize(constant.literal); + let uv = match ct { + mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(), + other => span_bug!(constant.span, "{other:#?}"), + }; + let c = self.cx.tcx().const_eval_resolve_for_typeck( + ty::ParamEnv::reveal_all(), + uv, + Some(constant.span), + ); let (llval, ty) = self.simd_shuffle_indices( &bx, constant.span, diff --git a/compiler/rustc_codegen_ssa/src/mir/constant.rs b/compiler/rustc_codegen_ssa/src/mir/constant.rs index 14fe84a146da0..a2ee9c54b16f9 100644 --- a/compiler/rustc_codegen_ssa/src/mir/constant.rs +++ b/compiler/rustc_codegen_ssa/src/mir/constant.rs @@ -65,16 +65,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx: &Bx, span: Span, ty: Ty<'tcx>, - constant: Result, ErrorHandled>, + constant: Result>, ErrorHandled>, ) -> (Bx::Value, Ty<'tcx>) { - constant + let val = constant + .ok() + .flatten() .map(|val| { let field_ty = ty.builtin_index().unwrap(); - let c = mir::ConstantKind::from_value(val, ty); - let values: Vec<_> = bx - .tcx() - .destructure_mir_constant(ty::ParamEnv::reveal_all(), c) - .fields + let values: Vec<_> = val + .unwrap_branch() .iter() .map(|field| { if let Some(prim) = field.try_to_scalar() { @@ -88,15 +87,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } }) .collect(); - let llval = bx.const_struct(&values, false); - (llval, c.ty()) + bx.const_struct(&values, false) }) - .unwrap_or_else(|_| { + .unwrap_or_else(|| { bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span }); // We've errored, so we don't have to produce working code. let ty = self.monomorphize(ty); let llty = bx.backend_type(bx.layout_of(ty)); - (bx.const_undef(llty), ty) - }) + bx.const_undef(llty) + }); + (val, ty) } } diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index b9ab0a4b7c8f4..5cc1fa2a4974b 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -92,7 +92,6 @@ pub(crate) fn try_destructure_mir_constant<'tcx>( param_env: ty::ParamEnv<'tcx>, val: mir::ConstantKind<'tcx>, ) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> { - trace!("destructure_mir_constant: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); let op = ecx.eval_mir_constant(&val, None, None)?; diff --git a/compiler/rustc_middle/src/mir/interpret/queries.rs b/compiler/rustc_middle/src/mir/interpret/queries.rs index ae32a54be3ded..9c97431f3614a 100644 --- a/compiler/rustc_middle/src/mir/interpret/queries.rs +++ b/compiler/rustc_middle/src/mir/interpret/queries.rs @@ -95,11 +95,15 @@ impl<'tcx> TyCtxt<'tcx> { // used generic parameters is a bug of evaluation, so checking for it // here does feel somewhat sensible. if !self.features().generic_const_exprs && ct.substs.has_non_region_param() { - assert!(matches!( - self.def_kind(ct.def), - DefKind::InlineConst | DefKind::AnonConst - )); - let mir_body = self.mir_for_ctfe(ct.def); + let def_kind = self.def_kind(instance.def_id()); + assert!( + matches!( + def_kind, + DefKind::InlineConst | DefKind::AnonConst | DefKind::AssocConst + ), + "{cid:?} is {def_kind:?}", + ); + let mir_body = self.mir_for_ctfe(instance.def_id()); if mir_body.is_polymorphic { let Some(local_def_id) = ct.def.as_local() else { return }; self.struct_span_lint_hir( @@ -239,15 +243,3 @@ impl<'tcx> TyCtxtEnsure<'tcx> { self.eval_to_allocation_raw(param_env.and(gid)) } } - -impl<'tcx> TyCtxt<'tcx> { - /// Destructure a mir constant ADT or array into its variant index and its field values. - /// Panics if the destructuring fails, use `try_destructure_mir_constant` for fallible version. - pub fn destructure_mir_constant( - self, - param_env: ty::ParamEnv<'tcx>, - constant: mir::ConstantKind<'tcx>, - ) -> mir::DestructuredConstant<'tcx> { - self.try_destructure_mir_constant(param_env.and(constant)).unwrap() - } -} diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs index 58590df1fedf8..ac49ccdb5011b 100644 --- a/src/tools/clippy/clippy_lints/src/non_copy_const.rs +++ b/src/tools/clippy/clippy_lints/src/non_copy_const.rs @@ -15,12 +15,14 @@ use rustc_hir::{ }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{LateContext, LateLintPass, Lint}; -use rustc_middle::mir; -use rustc_middle::mir::interpret::{ConstValue, ErrorHandled}; +use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, InnerSpan, Span}; +use rustc_target::abi::VariantIdx; +use rustc_middle::mir::interpret::EvalToValTreeResult; +use rustc_middle::mir::interpret::GlobalId; // FIXME: this is a correctness problem but there's no suitable // warn-by-default category. @@ -141,21 +143,35 @@ fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { fn is_value_unfrozen_raw<'tcx>( cx: &LateContext<'tcx>, - result: Result, ErrorHandled>, + result: Result>, ErrorHandled>, ty: Ty<'tcx>, ) -> bool { - fn inner<'tcx>(cx: &LateContext<'tcx>, val: mir::ConstantKind<'tcx>) -> bool { - match val.ty().kind() { + fn inner<'tcx>(cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool { + match *ty.kind() { // the fact that we have to dig into every structs to search enums // leads us to the point checking `UnsafeCell` directly is the only option. ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true, // As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the // contained value. ty::Adt(def, ..) if def.is_union() => false, - ty::Array(..) | ty::Adt(..) | ty::Tuple(..) => { - let val = cx.tcx.destructure_mir_constant(cx.param_env, val); - val.fields.iter().any(|field| inner(cx, *field)) + ty::Array(ty, _) => { + val.unwrap_branch().iter().any(|field| inner(cx, *field, ty)) }, + ty::Adt(def, _) if def.is_union() => false, + ty::Adt(def, substs) if def.is_enum() => { + let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap(); + let variant_index = + VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap()); + fields.iter().copied().zip( + def.variants()[variant_index] + .fields + .iter() + .map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, field, ty)) + } + ty::Adt(def, substs) => { + val.unwrap_branch().iter().zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, *field, ty)) + } + ty::Tuple(tys) => val.unwrap_branch().iter().zip(tys).any(|(field, ty)| inner(cx, *field, ty)), _ => false, } } @@ -184,24 +200,44 @@ fn is_value_unfrozen_raw<'tcx>( // I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none). err == ErrorHandled::TooGeneric }, - |val| inner(cx, mir::ConstantKind::from_value(val, ty)), + |val| val.map_or(true, |val| inner(cx, val, ty)), ) } fn is_value_unfrozen_poly<'tcx>(cx: &LateContext<'tcx>, body_id: BodyId, ty: Ty<'tcx>) -> bool { - let result = cx.tcx.const_eval_poly(body_id.hir_id.owner.to_def_id()); + let def_id = body_id.hir_id.owner.to_def_id(); + let substs = ty::InternalSubsts::identity_for_item(cx.tcx, def_id); + let instance = ty::Instance::new(def_id, substs); + let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None }; + let param_env = cx.tcx.param_env(def_id).with_reveal_all_normalized(cx.tcx); + let result = cx.tcx.const_eval_global_id_for_typeck(param_env, cid, None); is_value_unfrozen_raw(cx, result, ty) } fn is_value_unfrozen_expr<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId, def_id: DefId, ty: Ty<'tcx>) -> bool { let substs = cx.typeck_results().node_substs(hir_id); - let result = cx - .tcx - .const_eval_resolve(cx.param_env, mir::UnevaluatedConst::new(def_id, substs), None); + let result = const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, substs), None); is_value_unfrozen_raw(cx, result, ty) } + +pub fn const_eval_resolve<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ct: ty::UnevaluatedConst<'tcx>, + span: Option, +) -> EvalToValTreeResult<'tcx> { + match ty::Instance::resolve(tcx, param_env, ct.def, ct.substs) { + Ok(Some(instance)) => { + let cid = GlobalId { instance, promoted: None }; + tcx.const_eval_global_id_for_typeck(param_env, cid, span) + } + Ok(None) => Err(ErrorHandled::TooGeneric), + Err(err) => Err(ErrorHandled::Reported(err.into())), + } +} + #[derive(Copy, Clone)] enum Source { Item { item: Span }, diff --git a/src/tools/clippy/tests/ui/crashes/ice-9445.stderr b/src/tools/clippy/tests/ui/crashes/ice-9445.stderr new file mode 100644 index 0000000000000..a59d098e5ac0a --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-9445.stderr @@ -0,0 +1,12 @@ +error: a `const` item should never be interior mutable + --> $DIR/ice-9445.rs:1:1 + | +LL | const UNINIT: core::mem::MaybeUninit> = core::mem::MaybeUninit::uninit(); + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | make this a static item (maybe with lazy_static) + | + = note: `-D clippy::declare-interior-mutable-const` implied by `-D warnings` + +error: aborting due to previous error +