diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs index 4713c99d33b5..76b6994e5d09 100644 --- a/clippy_lints/src/methods/is_empty.rs +++ b/clippy_lints/src/methods/is_empty.rs @@ -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; @@ -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 { - 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 - } -} diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 61b38391d9e0..fd136827c604 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -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}; @@ -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 { + 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>, @@ -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() { @@ -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 { + 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> { use self::Constant::{Bool, Int}; @@ -515,8 +561,11 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { vec.iter().map(|elem| self.expr(elem)).collect::>() } - /// Lookup a possibly constant expression from an `ExprKind::Path`. - fn fetch_path(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>) -> Option> { + /// Lookup a possibly constant expression from an `ExprKind::Path` and apply a function on it. + fn fetch_path_and_apply(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>, f: F) -> Option + where + F: FnOnce(&mut Self, rustc_middle::mir::Const<'tcx>) -> Option, + { let res = self.typeck_results.qpath_res(qpath, id); match res { Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => { @@ -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, } @@ -742,7 +789,6 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option> { - use rustc_middle::mir::ConstValue; let mir::Const::Val(val, _) = result else { // We only work on evaluated consts. return None; @@ -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 { + 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>, diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs index d61cdbb04249..bd7e3a7c5d5b 100644 --- a/tests/ui/const_is_empty.rs +++ b/tests/ui/const_is_empty.rs @@ -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(); diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr index 5a89e686242d..ef5aa556e6a0 100644 --- a/tests/ui/const_is_empty.stderr +++ b/tests/ui/const_is_empty.stderr @@ -4,11 +4,6 @@ error: this expression always evaluates to true LL | if "".is_empty() { | ^^^^^^^^^^^^^ | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:4:8 - | -LL | if "".is_empty() { - | ^^ = note: `-D clippy::const-is-empty` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::const_is_empty)]` @@ -17,108 +12,144 @@ error: this expression always evaluates to false | LL | if "foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:7:8 - | -LL | if "foobar".is_empty() { - | ^^^^^^^^ error: this expression always evaluates to true --> tests/ui/const_is_empty.rs:13:8 | LL | if b"".is_empty() { | ^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:13:8 - | -LL | if b"".is_empty() { - | ^^^ error: this expression always evaluates to false --> tests/ui/const_is_empty.rs:16:8 | LL | if b"foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:16:8 - | -LL | if b"foobar".is_empty() { - | ^^^^^^^^^ error: this expression always evaluates to true --> tests/ui/const_is_empty.rs:33:8 | LL | if empty2.is_empty() { | ^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:29:17 - | -LL | let empty = ""; - | ^^ error: this expression always evaluates to false --> tests/ui/const_is_empty.rs:36:8 | LL | if non_empty2.is_empty() { | ^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:58:13 | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:30:21 +LL | let _ = EMPTY_STR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:60:13 | -LL | let non_empty = "foobar"; - | ^^^^^^^^ +LL | let _ = NON_EMPTY_STR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:62:13 + | +LL | let _ = EMPTY_BSTR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:43:13 + --> tests/ui/const_is_empty.rs:64:13 | -LL | let _ = value.is_empty(); - | ^^^^^^^^^^^^^^^^ +LL | let _ = NON_EMPTY_BSTR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:66:13 + | +LL | let _ = EMPTY_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:68:13 | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:42:17 +LL | let _ = EMPTY_ARRAY_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:70:13 | -LL | let value = "foobar"; - | ^^^^^^^^ +LL | let _ = EMPTY_U8_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:46:13 + --> tests/ui/const_is_empty.rs:72:13 | -LL | let _ = x.is_empty(); - | ^^^^^^^^^^^^ +LL | let _ = NON_EMPTY_U8_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:74:13 | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:42:17 +LL | let _ = NON_EMPTY_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:76:13 | -LL | let value = "foobar"; - | ^^^^^^^^ +LL | let _ = NON_EMPTY_ARRAY_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:48:13 + --> tests/ui/const_is_empty.rs:78:13 | -LL | let _ = "".is_empty(); - | ^^^^^^^^^^^^^ +LL | let _ = EMPTY_REF_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:80:13 + | +LL | let _ = NON_EMPTY_REF_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:82:13 + | +LL | let _ = EMPTY_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:84:13 + | +LL | let _ = NON_EMPTY_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:86:13 | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:48:13 +LL | let _ = NON_EMPTY_SLICE_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:92:13 + | +LL | let _ = value.is_empty(); + | ^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:95:13 + | +LL | let _ = x.is_empty(); + | ^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:97:13 | LL | let _ = "".is_empty(); - | ^^ + | ^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:50:13 + --> tests/ui/const_is_empty.rs:99:13 | LL | let _ = b"".is_empty(); | ^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:50:13 - | -LL | let _ = b"".is_empty(); - | ^^^ -error: aborting due to 10 previous errors +error: aborting due to 25 previous errors