Skip to content

Commit

Permalink
[arithmetic_side_effects] Fix rust-lang#10792
Browse files Browse the repository at this point in the history
  • Loading branch information
@c410-f3r
c410-f3r committed May 24, 2023
1 parent 5187e92 commit 8f1ed40
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 57 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/enum_clike.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'tcx> LateLintPass<'tcx> for UnportableVariant {
.const_eval_poly(def_id.to_def_id())
.ok()
.map(|val| rustc_middle::mir::ConstantKind::from_value(val, ty));
if let Some(Constant::Int(val)) = constant.and_then(|c| miri_to_const(cx.tcx, c)) {
if let Some(Constant::Int(val)) = constant.and_then(|c| miri_to_const(cx, c, None)) {
if let ty::Adt(adt, _) = ty.kind() {
if adt.is_enum() {
ty = adt.repr().discr_type().to_ty(cx.tcx);
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/matches/overlapping_arms.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>)
cx.tcx.valtree_to_const_val((ty, min_val_const.to_valtree())),
ty,
);
miri_to_const(cx.tcx, min_constant)?
miri_to_const(cx, min_constant, None)?
},
};
let rhs_const = match rhs {
Expand All @@ -52,7 +52,7 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>)
cx.tcx.valtree_to_const_val((ty, max_val_const.to_valtree())),
ty,
);
miri_to_const(cx.tcx, max_constant)?
miri_to_const(cx, max_constant, None)?
},
};
let lhs_val = lhs_const.int_value(cx, ty)?;
Expand Down
132 changes: 80 additions & 52 deletions clippy_utils/src/consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::ty::{self, EarlyBinder, FloatTy, ScalarInt, Ty, TyCtxt};
use rustc_middle::ty::{List, SubstsRef};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::SyntaxContext;
use std::cmp::Ordering::{self, Equal};
use std::hash::{Hash, Hasher};
Expand Down Expand Up @@ -249,7 +249,7 @@ pub fn constant<'tcx>(
typeck_results: &ty::TypeckResults<'tcx>,
e: &Expr<'_>,
) -> Option<Constant> {
ConstEvalLateContext::new(lcx, typeck_results).expr(e)
ConstEvalLateContext::new(lcx, typeck_results).expr(e, None)
}

/// Attempts to evaluate the expression as a constant.
Expand All @@ -259,7 +259,7 @@ pub fn constant_with_source<'tcx>(
e: &Expr<'_>,
) -> Option<(Constant, ConstantSource)> {
let mut ctxt = ConstEvalLateContext::new(lcx, typeck_results);
let res = ctxt.expr(e);
let res = ctxt.expr(e, None);
res.map(|x| (x, ctxt.source))
}

Expand Down Expand Up @@ -338,9 +338,9 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}

/// Simple constant folding: Insert an expression, get a constant or none.
pub fn expr(&mut self, e: &Expr<'_>) -> Option<Constant> {
pub fn expr(&mut self, e: &Expr<'_>, field: Option<Ident>) -> Option<Constant> {
match e.kind {
ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e)),
ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e), field),
ExprKind::Block(block, _) => self.block(block),
ExprKind::Lit(lit) => {
if is_direct_expn_of(e.span, "cfg").is_some() {
Expand All @@ -356,9 +356,9 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
ty::Array(_, n) => n.try_eval_target_usize(self.lcx.tcx, self.lcx.param_env)?,
_ => span_bug!(e.span, "typeck error"),
};
self.expr(value).map(|v| Constant::Repeat(Box::new(v), n))
self.expr(value, None).map(|v| Constant::Repeat(Box::new(v), n))
},
ExprKind::Unary(op, operand) => self.expr(operand).and_then(|o| match op {
ExprKind::Unary(op, operand) => self.expr(operand, None).and_then(|o| match op {
UnOp::Not => self.constant_not(&o, self.typeck_results.expr_ty(e)),
UnOp::Neg => self.constant_negate(&o, self.typeck_results.expr_ty(e)),
UnOp::Deref => Some(if let Constant::Ref(r) = o { *r } else { o }),
Expand Down Expand Up @@ -391,8 +391,8 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}
},
ExprKind::Index(arr, index) => self.index(arr, index),
ExprKind::AddrOf(_, _, inner) => self.expr(inner).map(|r| Constant::Ref(Box::new(r))),
// TODO: add other expressions.
ExprKind::AddrOf(_, _, inner) => self.expr(inner, None).map(|r| Constant::Ref(Box::new(r))),
ExprKind::Field(local_expr, field) => self.expr(local_expr, Some(field)),
_ => None,
}
}
Expand Down Expand Up @@ -434,27 +434,24 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
/// Create `Some(Vec![..])` of all constants, unless there is any
/// non-constant part.
fn multi(&mut self, vec: &[Expr<'_>]) -> Option<Vec<Constant>> {
vec.iter().map(|elem| self.expr(elem)).collect::<Option<_>>()
vec.iter().map(|elem| self.expr(elem, None)).collect::<Option<_>>()
}

/// Lookup a possibly constant expression from an `ExprKind::Path`.
fn fetch_path(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>) -> Option<Constant> {
fn fetch_path(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>, field: Option<Ident>) -> Option<Constant> {
let res = self.typeck_results.qpath_res(qpath, id);
match res {
Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => {
// Check if this constant is based on `cfg!(..)`,
// which is NOT constant for our purposes.
if let Some(node) = self.lcx.tcx.hir().get_if_local(def_id) &&
let Node::Item(&Item {
kind: ItemKind::Const(_, body_id),
..
}) = node &&
let Node::Expr(&Expr {
kind: ExprKind::Lit(_),
span,
..
}) = self.lcx.tcx.hir().get(body_id.hir_id) &&
is_direct_expn_of(span, "cfg").is_some() {
if let Some(node) = self.lcx.tcx.hir().get_if_local(def_id)
&& let Node::Item(Item { kind: ItemKind::Const(_, body_id), .. }) = node
&& let Node::Expr(Expr { kind: ExprKind::Lit(_), span, .. }) = self.lcx
.tcx
.hir()
.get(body_id.hir_id)
&& is_direct_expn_of(*span, "cfg").is_some()
{
return None;
}

Expand All @@ -464,25 +461,23 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
} else {
EarlyBinder(substs).subst(self.lcx.tcx, self.substs)
};

let result = self
.lcx
.tcx
.const_eval_resolve(self.param_env, mir::UnevaluatedConst::new(def_id, substs), None)
.ok()
.map(|val| rustc_middle::mir::ConstantKind::from_value(val, ty))?;
let result = miri_to_const(self.lcx.tcx, result)?;
let result = miri_to_const(self.lcx, result, field)?;
self.source = ConstantSource::Constant;
Some(result)
},
// FIXME: cover all usable cases.
_ => None,
}
}

fn index(&mut self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option<Constant> {
let lhs = self.expr(lhs);
let index = self.expr(index);
let lhs = self.expr(lhs, None);
let index = self.expr(index, None);

match (lhs, index) {
(Some(Constant::Vec(vec)), Some(Constant::Int(index))) => match vec.get(index as usize) {
Expand Down Expand Up @@ -533,27 +528,27 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}
}

self.expr(expr)
self.expr(expr, None)
} else {
None
}
}

fn ifthenelse(&mut self, cond: &Expr<'_>, then: &Expr<'_>, otherwise: Option<&Expr<'_>>) -> Option<Constant> {
if let Some(Constant::Bool(b)) = self.expr(cond) {
if let Some(Constant::Bool(b)) = self.expr(cond, None) {
if b {
self.expr(then)
self.expr(then, None)
} else {
otherwise.as_ref().and_then(|expr| self.expr(expr))
otherwise.as_ref().and_then(|expr| self.expr(expr, None))
}
} else {
None
}
}

fn binop(&mut self, op: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> Option<Constant> {
let l = self.expr(left)?;
let r = self.expr(right);
let l = self.expr(left, None)?;
let r = self.expr(right, None);
match (l, r) {
(Constant::Int(l), Some(Constant::Int(r))) => match *self.typeck_results.expr_ty_opt(left)?.kind() {
ty::Int(ity) => {
Expand Down Expand Up @@ -644,23 +639,31 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}
}

pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -> Option<Constant> {
pub fn miri_to_const<'tcx>(
lcx: &LateContext<'tcx>,
result: mir::ConstantKind<'tcx>,
field: Option<Ident>,
) -> Option<Constant> {
use rustc_middle::mir::interpret::ConstValue;
match result {
mir::ConstantKind::Val(ConstValue::Scalar(Scalar::Int(int)), _) => {
match result.ty().kind() {
ty::Bool => Some(Constant::Bool(int == ScalarInt::TRUE)),
ty::Uint(_) | ty::Int(_) => Some(Constant::Int(int.assert_bits(int.size()))),
ty::Float(FloatTy::F32) => Some(Constant::F32(f32::from_bits(
int.try_into().expect("invalid f32 bit representation"),
))),
ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits(
int.try_into().expect("invalid f64 bit representation"),
))),
ty::RawPtr(_) => Some(Constant::RawPtr(int.assert_bits(int.size()))),
// FIXME: implement other conversions.
_ => None,
}
mir::ConstantKind::Val(ConstValue::Scalar(Scalar::Int(int)), _) => match result.ty().kind() {
ty::Adt(adt_def, _) if adt_def.is_struct() => {
if let Some(desired_field) = field_of_struct(*adt_def, lcx, result, field) {
miri_to_const(lcx, desired_field, None)
} else {
None
}
},
ty::Bool => Some(Constant::Bool(int == ScalarInt::TRUE)),
ty::Uint(_) | ty::Int(_) => Some(Constant::Int(int.assert_bits(int.size()))),
ty::Float(FloatTy::F32) => Some(Constant::F32(f32::from_bits(
int.try_into().expect("invalid f32 bit representation"),
))),
ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits(
int.try_into().expect("invalid f64 bit representation"),
))),
ty::RawPtr(_) => Some(Constant::RawPtr(int.assert_bits(int.size()))),
_ => None,
},
mir::ConstantKind::Val(ConstValue::Slice { data, start, end }, _) => match result.ty().kind() {
ty::Ref(_, tam, _) => match tam.kind() {
Expand All @@ -676,8 +679,15 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
_ => None,
},
mir::ConstantKind::Val(ConstValue::ByRef { alloc, offset: _ }, _) => match result.ty().kind() {
ty::Adt(adt_def, _) if adt_def.is_struct() => {
if let Some(desired_field) = field_of_struct(*adt_def, lcx, result, field) {
miri_to_const(lcx, desired_field, None)
} else {
None
}
},
ty::Array(sub_type, len) => match sub_type.kind() {
ty::Float(FloatTy::F32) => match len.kind().try_to_target_usize(tcx) {
ty::Float(FloatTy::F32) => match len.kind().try_to_target_usize(lcx.tcx) {
Some(len) => alloc
.inner()
.inspect_with_uninit_and_ptr_outside_interpreter(0..(4 * usize::try_from(len).unwrap()))
Expand All @@ -688,7 +698,7 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
.map(Constant::Vec),
_ => None,
},
ty::Float(FloatTy::F64) => match len.kind().try_to_target_usize(tcx) {
ty::Float(FloatTy::F64) => match len.kind().try_to_target_usize(lcx.tcx) {
Some(len) => alloc
.inner()
.inspect_with_uninit_and_ptr_outside_interpreter(0..(8 * usize::try_from(len).unwrap()))
Expand All @@ -699,12 +709,30 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
.map(Constant::Vec),
_ => None,
},
// FIXME: implement other array type conversions.
_ => None,
},
_ => None,
},
// FIXME: implement other conversions.
_ => None,
}
}

fn field_of_struct<'tcx>(
adt_def: ty::AdtDef<'tcx>,
lcx: &LateContext<'tcx>,
result: mir::ConstantKind<'tcx>,
field: Option<Ident>,
) -> Option<mir::ConstantKind<'tcx>> {
let dc = lcx.tcx.destructure_mir_constant(lcx.param_env, result);
if let Some(dc_variant) = dc.variant
&& let Some(local_field) = field
&& let Some(variant) = adt_def.variants().get(dc_variant)
&& let Some(field_idx) = variant.fields.iter().position(|el| el.name == local_field.name)
&& let Some(dc_field) = dc.fields.get(field_idx)
{
Some(*dc_field)
}
else {
None
}
}
4 changes: 2 additions & 2 deletions clippy_utils/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti
&& let Some(min_val) = bnd_ty.numeric_min_val(cx.tcx)
&& let const_val = cx.tcx.valtree_to_const_val((bnd_ty, min_val.to_valtree()))
&& let min_const_kind = ConstantKind::from_value(const_val, bnd_ty)
&& let Some(min_const) = miri_to_const(cx.tcx, min_const_kind)
&& let Some(min_const) = miri_to_const(cx, min_const_kind, None)
&& let Some(start_const) = constant(cx, cx.typeck_results(), start)
{
start_const == min_const
Expand All @@ -1514,7 +1514,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti
&& let Some(max_val) = bnd_ty.numeric_max_val(cx.tcx)
&& let const_val = cx.tcx.valtree_to_const_val((bnd_ty, max_val.to_valtree()))
&& let max_const_kind = ConstantKind::from_value(const_val, bnd_ty)
&& let Some(max_const) = miri_to_const(cx.tcx, max_const_kind)
&& let Some(max_const) = miri_to_const(cx, max_const_kind, None)
&& let Some(end_const) = constant(cx, cx.typeck_results(), end)
{
end_const == max_const
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/arithmetic_side_effects.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,4 +466,19 @@ pub fn issue_10767() {
&3.5_f32 + &1.3_f32;
}

pub fn issue_10792() {
struct One {
a: u32,
}
struct Two {
b: u32,
c: u64,
}
const ONE: One = One { a: 1 };
const TWO: Two = Two { b: 2, c: 3 };
let _ = 10 / ONE.a;
let _ = 10 / TWO.b;
let _ = 10 / TWO.c;
}

fn main() {}

0 comments on commit 8f1ed40

Please sign in to comment.