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 c490974 commit fa54b19
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 58 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
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/repeat_once.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(super) fn check<'tcx>(
recv: &'tcx Expr<'_>,
repeat_arg: &'tcx Expr<'_>,
) {
if constant_context(cx, cx.typeck_results()).expr(repeat_arg) == Some(Constant::Int(1)) {
if constant_context(cx, cx.typeck_results()).expr(repeat_arg, None) == Some(Constant::Int(1)) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
if ty.is_str() {
span_lint_and_sugg(
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/transmute/transmuting_null.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t
// Catching transmute over constants that resolve to `null`.
let mut const_eval_context = constant_context(cx, cx.typeck_results());
if let ExprKind::Path(ref _qpath) = arg.kind &&
let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg)
let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg, None)
{
span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG);
return true;
Expand Down
115 changes: 64 additions & 51 deletions clippy_utils/src/consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::ty::SubstsRef;
use rustc_middle::ty::{self, EarlyBinder, FloatTy, ScalarInt, Ty, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{Ident, Symbol};
use std::cmp::Ordering::{self, Equal};
use std::hash::{Hash, Hasher};
use std::iter;
Expand Down Expand Up @@ -239,7 +239,7 @@ pub fn constant<'tcx>(
needed_resolution: false,
substs: ty::List::empty(),
};
cx.expr(e).map(|cst| (cst, cx.needed_resolution))
cx.expr(e, None).map(|cst| (cst, cx.needed_resolution))
}

pub fn constant_simple<'tcx>(
Expand Down Expand Up @@ -320,9 +320,9 @@ pub struct ConstEvalLateContext<'a, 'tcx> {

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 @@ -338,9 +338,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 @@ -373,8 +373,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 @@ -416,27 +416,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 @@ -446,27 +443,25 @@ 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);
if result.is_some() {
self.needed_resolution = true;
}
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 All @@ -492,27 +487,27 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
/// A block can only yield a constant if it only has one constant expression.
fn block(&mut self, block: &Block<'_>) -> Option<Constant> {
if block.stmts.is_empty() {
block.expr.as_ref().and_then(|b| self.expr(b))
block.expr.as_ref().and_then(|b| self.expr(b, 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 @@ -603,23 +598,29 @@ 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() => {
let dc = lcx.tcx.destructure_mir_constant(lcx.param_env, result);
let &[field] = dc.fields else { return None; };
miri_to_const(lcx, field, 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 @@ -635,8 +636,22 @@ 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() => {
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)
{
miri_to_const(lcx, *dc_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 @@ -647,7 +662,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 @@ -658,12 +673,10 @@ 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,
}
}
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 @@ -1497,7 +1497,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 @@ -1513,7 +1513,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 fa54b19

Please sign in to comment.