[arithmetic-side-effects] Finish non-overflowing ops
This commit is contained in:
parent
56a8ef4dbe
commit
dba5adae6e
3 changed files with 127 additions and 82 deletions
|
|
@ -42,26 +42,23 @@ impl ArithmeticSideEffects {
|
|||
}
|
||||
}
|
||||
|
||||
/// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
|
||||
/// won't overflow.
|
||||
fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
|
||||
if !Self::is_literal_integer(rhs, rhs_refs) {
|
||||
return false;
|
||||
}
|
||||
/// Assuming that `expr` is a literal integer, checks assign operators (+=, -=, *=, /=) in a
|
||||
/// non-constant environment that won't overflow.
|
||||
fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
|
||||
if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node
|
||||
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
|
||||
&& let hir::ExprKind::Lit(ref lit) = expr.kind
|
||||
&& let ast::LitKind::Int(0, _) = lit.node
|
||||
{
|
||||
return true;
|
||||
}
|
||||
if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node
|
||||
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
|
||||
&& let hir::ExprKind::Lit(ref lit) = expr.kind
|
||||
&& !matches!(lit.node, ast::LitKind::Int(0, _))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
if let hir::BinOpKind::Mul = op.node
|
||||
&& let hir::ExprKind::Lit(ref lit) = rhs.kind
|
||||
&& let hir::ExprKind::Lit(ref lit) = expr.kind
|
||||
&& let ast::LitKind::Int(0 | 1, _) = lit.node
|
||||
{
|
||||
return true;
|
||||
|
|
@ -69,12 +66,6 @@ impl ArithmeticSideEffects {
|
|||
false
|
||||
}
|
||||
|
||||
/// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
|
||||
/// already handled by the CTFE.
|
||||
fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
|
||||
Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
|
||||
}
|
||||
|
||||
/// Checks if the given `expr` has any of the inner `allowed` elements.
|
||||
fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
|
||||
self.allowed.contains(
|
||||
|
|
@ -95,7 +86,8 @@ impl ArithmeticSideEffects {
|
|||
}
|
||||
|
||||
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
|
||||
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected");
|
||||
let msg = "arithmetic operation that can potentially result in unexpected side-effects";
|
||||
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
|
||||
self.expr_span = Some(expr.span);
|
||||
}
|
||||
|
||||
|
|
@ -127,13 +119,18 @@ impl ArithmeticSideEffects {
|
|||
if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
|
||||
return;
|
||||
}
|
||||
let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
|
||||
let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
|
||||
let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
|
||||
if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
|
||||
return;
|
||||
let has_valid_op = match (
|
||||
Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
|
||||
Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
|
||||
) {
|
||||
(true, true) => true,
|
||||
(true, false) => Self::has_valid_assign_op(op, lhs),
|
||||
(false, true) => Self::has_valid_assign_op(op, rhs),
|
||||
(false, false) => false,
|
||||
};
|
||||
if !has_valid_op {
|
||||
self.issue_lint(cx, expr);
|
||||
}
|
||||
self.issue_lint(cx, expr);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue