Skip to content

Commit

Permalink
feat: extend const_is_empty with many kinds of constants
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Feb 19, 2024
1 parent 67db984 commit c94ef81
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 90 deletions.
27 changes: 5 additions & 22 deletions clippy_lints/src/methods/is_empty.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::consts::constant_is_empty;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::expr_or_init;
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind};
use rustc_hir::Expr;
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_span::source_map::Spanned;

use super::CONST_IS_EMPTY;

Expand All @@ -13,28 +12,12 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_
return;
}
let init_expr = expr_or_init(cx, receiver);
if let Some(init_is_empty) = is_empty(init_expr)
&& init_expr.span.eq_ctxt(receiver.span)
{
span_lint_and_note(
if let Some(init_is_empty) = constant_is_empty(cx, init_expr) {
span_lint(
cx,
CONST_IS_EMPTY,
expr.span,
&format!("this expression always evaluates to {init_is_empty:?}"),
Some(init_expr.span),
"because its initialization value is constant",
);
}
}

fn is_empty(expr: &'_ rustc_hir::Expr<'_>) -> Option<bool> {
if let ExprKind::Lit(Spanned { node, .. }) = expr.kind {
match node {
LitKind::Str(sym, _) => Some(sym.is_empty()),
LitKind::ByteStr(value, _) | LitKind::CStr(value, _) => Some(value.is_empty()),
_ => None,
}
} else {
None
}
}
94 changes: 87 additions & 7 deletions clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::{BinOp, BinOpKind, Block, ConstBlock, Expr, ExprKind, HirId, Item
use rustc_lexer::tokenize;
use rustc_lint::LateContext;
use rustc_middle::mir::interpret::{alloc_range, Scalar};
use rustc_middle::mir::ConstValue;
use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List, ScalarInt, Ty, TyCtxt, UintTy};
use rustc_middle::{bug, mir, span_bug};
use rustc_span::symbol::{Ident, Symbol};
Expand Down Expand Up @@ -303,6 +304,12 @@ impl ConstantSource {
}
}

/// Attempts to check whether the expression is a constant representing an empty slice, str, array,
/// etc…
pub fn constant_is_empty(lcx: &LateContext<'_>, e: &Expr<'_>) -> Option<bool> {
ConstEvalLateContext::new(lcx, lcx.typeck_results()).expr_is_empty(e)
}

/// Attempts to evaluate the expression as a constant.
pub fn constant<'tcx>(
lcx: &LateContext<'tcx>,
Expand Down Expand Up @@ -402,7 +409,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
match e.kind {
ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr(self.lcx.tcx.hir().body(body).value),
ExprKind::DropTemps(e) => self.expr(e),
ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e)),
ExprKind::Path(ref qpath) => {
self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| {
let result = mir_to_const(this.lcx, result)?;
this.source = ConstantSource::Constant;
Some(result)
})
},
ExprKind::Block(block, _) => self.block(block),
ExprKind::Lit(lit) => {
if is_direct_expn_of(e.span, "cfg").is_some() {
Expand Down Expand Up @@ -468,6 +481,39 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}
}

/// Simple constant folding to determine if an expression is an empty slice, str, array, …
pub fn expr_is_empty(&mut self, e: &Expr<'_>) -> Option<bool> {
match e.kind {
ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr_is_empty(self.lcx.tcx.hir().body(body).value),
ExprKind::DropTemps(e) => self.expr_is_empty(e),
ExprKind::Path(ref qpath) => {
self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| {
mir_is_empty(this.lcx, result)
})
},
ExprKind::Lit(lit) => {
if is_direct_expn_of(e.span, "cfg").is_some() {
None
} else {
match &lit.node {
LitKind::Str(is, _) => Some(is.is_empty()),
LitKind::ByteStr(s, _) | LitKind::CStr(s, _) => Some(s.is_empty()),
_ => None,
}
}
},
ExprKind::Array(vec) => self.multi(vec).map(|v| v.is_empty()),
ExprKind::Repeat(..) => {
if let ty::Array(_, n) = self.typeck_results.expr_ty(e).kind() {
Some(n.try_eval_target_usize(self.lcx.tcx, self.lcx.param_env)? == 0)
} else {
span_bug!(e.span, "typeck error");
}
},
_ => None,
}
}

#[expect(clippy::cast_possible_wrap)]
fn constant_not(&self, o: &Constant<'tcx>, ty: Ty<'_>) -> Option<Constant<'tcx>> {
use self::Constant::{Bool, Int};
Expand Down Expand Up @@ -515,8 +561,11 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
vec.iter().map(|elem| self.expr(elem)).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<'tcx>> {
/// Lookup a possibly constant expression from an `ExprKind::Path` and apply a function on it.
fn fetch_path_and_apply<T, F>(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>, f: F) -> Option<T>
where
F: FnOnce(&mut Self, rustc_middle::mir::Const<'tcx>) -> Option<T>,
{
let res = self.typeck_results.qpath_res(qpath, id);
match res {
Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => {
Expand Down Expand Up @@ -549,9 +598,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
.const_eval_resolve(self.param_env, mir::UnevaluatedConst::new(def_id, args), None)
.ok()
.map(|val| rustc_middle::mir::Const::from_value(val, ty))?;
let result = mir_to_const(self.lcx, result)?;
self.source = ConstantSource::Constant;
Some(result)
f(self, result)
},
_ => None,
}
Expand Down Expand Up @@ -742,7 +789,6 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
}

pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option<Constant<'tcx>> {
use rustc_middle::mir::ConstValue;
let mir::Const::Val(val, _) = result else {
// We only work on evaluated consts.
return None;
Expand Down Expand Up @@ -788,6 +834,40 @@ pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) ->
}
}

fn mir_is_empty<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option<bool> {
let mir::Const::Val(val, _) = result else {
// We only work on evaluated consts.
return None;
};
match (val, result.ty().kind()) {
(_, ty::Ref(_, inner_ty, _)) => match inner_ty.kind() {
ty::Str | ty::Slice(_) => {
if let ConstValue::Indirect { alloc_id, offset } = val {
let a = lcx.tcx.global_alloc(alloc_id).unwrap_memory().inner();
let ptr_size = lcx.tcx.data_layout.pointer_size;
if a.size() < offset + 2 * ptr_size {
// (partially) dangling reference
return None;
}
let len = a
.read_scalar(&lcx.tcx, alloc_range(offset + ptr_size, ptr_size), false)
.ok()?
.to_target_usize(&lcx.tcx)
.ok()?;
Some(len == 0)
} else {
None
}
},
ty::Array(_, len) => Some(len.try_to_target_usize(lcx.tcx)? == 0),
_ => None,
},
(ConstValue::Indirect { .. }, ty::Array(_, len)) => Some(len.try_to_target_usize(lcx.tcx)? == 0),
(ConstValue::ZeroSized, _) => Some(true),
_ => None,
}
}

fn field_of_struct<'tcx>(
adt_def: ty::AdtDef<'tcx>,
lcx: &LateContext<'tcx>,
Expand Down
49 changes: 49 additions & 0 deletions tests/ui/const_is_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,55 @@ fn test_propagated() {
}
}

const EMPTY_STR: &str = "";
const NON_EMPTY_STR: &str = "foo";
const EMPTY_BSTR: &[u8] = b"";
const NON_EMPTY_BSTR: &[u8] = b"foo";
const EMPTY_U8_SLICE: &[u8] = &[];
const NON_EMPTY_U8_SLICE: &[u8] = &[1, 2];
const EMPTY_SLICE: &[u32] = &[];
const NON_EMPTY_SLICE: &[u32] = &[1, 2];
const NON_EMPTY_SLICE_REPEAT: &[u32] = &[1; 2];
const EMPTY_ARRAY: [u32; 0] = [];
const EMPTY_ARRAY_REPEAT: [u32; 0] = [1; 0];
const NON_EMPTY_ARRAY: [u32; 2] = [1, 2];
const NON_EMPTY_ARRAY_REPEAT: [u32; 2] = [1; 2];
const EMPTY_REF_ARRAY: &[u32; 0] = &[];
const NON_EMPTY_REF_ARRAY: &[u32; 3] = &[1, 2, 3];

fn test_from_const() {
let _ = EMPTY_STR.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = NON_EMPTY_STR.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = EMPTY_BSTR.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = NON_EMPTY_BSTR.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = EMPTY_ARRAY.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = EMPTY_ARRAY_REPEAT.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = EMPTY_U8_SLICE.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = NON_EMPTY_U8_SLICE.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = NON_EMPTY_ARRAY.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = NON_EMPTY_ARRAY_REPEAT.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = EMPTY_REF_ARRAY.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = NON_EMPTY_REF_ARRAY.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = EMPTY_SLICE.is_empty();
//~^ ERROR: this expression always evaluates to true
let _ = NON_EMPTY_SLICE.is_empty();
//~^ ERROR: this expression always evaluates to false
let _ = NON_EMPTY_SLICE_REPEAT.is_empty();
//~^ ERROR: this expression always evaluates to false
}

fn main() {
let value = "foobar";
let _ = value.is_empty();
Expand Down
Loading

0 comments on commit c94ef81

Please sign in to comment.