diff --git a/clippy_lints/src/zero_repeat_side_effects.rs b/clippy_lints/src/zero_repeat_side_effects.rs index ad041e55b..fa1cc41d1 100644 --- a/clippy_lints/src/zero_repeat_side_effects.rs +++ b/clippy_lints/src/zero_repeat_side_effects.rs @@ -3,10 +3,11 @@ use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet; use clippy_utils::visitors::for_each_expr_without_closures; use rustc_ast::LitKind; +use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; -use rustc_hir::{ExprKind, Node}; +use rustc_hir::{ArrayLen, ExprKind, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, ConstKind, Ty}; +use rustc_middle::ty::Ty; use rustc_session::declare_lint_pass; use rustc_span::Span; @@ -45,18 +46,26 @@ declare_clippy_lint! { declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]); impl LateLintPass<'_> for ZeroRepeatSideEffects { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &rustc_hir::Expr<'_>) { + let hir_map = cx.tcx.hir(); if let Some(args) = VecArgs::hir(cx, expr) && let VecArgs::Repeat(inner_expr, len) = args && let ExprKind::Lit(l) = len.kind - && let LitKind::Int(i, _) = l.node - && i.0 == 0 + && let LitKind::Int(Pu128(0), _) = l.node { inner_check(cx, expr, inner_expr, true); - } else if let ExprKind::Repeat(inner_expr, _) = expr.kind - && let ty::Array(_, cst) = cx.typeck_results().expr_ty(expr).kind() - && let ConstKind::Value(_, ty::ValTree::Leaf(element_count)) = cst.kind() - && element_count.to_target_usize(cx.tcx) == 0 + } + // Lint only if the length is a literal zero, and not a path to any constants. + // NOTE(@y21): When reading `[f(); LEN]`, I intuitively expect that the function is called and it + // doesn't seem as confusing as `[f(); 0]`. It would also have false positives when eg. + // the const item depends on `#[cfg]s` and has different values in different compilation + // sessions). + else if let ExprKind::Repeat(inner_expr, length) = expr.kind + && let ArrayLen::Body(anon_const) = length + && let length_expr = hir_map.body(anon_const.body).value + && !length_expr.span.from_expansion() + && let ExprKind::Lit(literal) = length_expr.kind + && let LitKind::Int(Pu128(0), _) = literal.node { inner_check(cx, expr, inner_expr, false); } diff --git a/tests/ui/zero_repeat_side_effects.fixed b/tests/ui/zero_repeat_side_effects.fixed index 6f1325219..2f71d1752 100644 --- a/tests/ui/zero_repeat_side_effects.fixed +++ b/tests/ui/zero_repeat_side_effects.fixed @@ -16,10 +16,8 @@ fn main() { // on arrays f(); let a: [i32; 0] = []; - f(); let a: [i32; 0] = []; let mut b; f(); b = [] as [i32; 0]; - f(); b = [] as [i32; 0]; // on vecs // vecs dont support infering value of consts @@ -39,9 +37,11 @@ fn main() { // when singled out/not part of assignment/local { f(); vec![] as std::vec::Vec }; { f(); [] as [i32; 0] }; - { f(); [] as [i32; 0] }; // should not trigger + let a = [f(); N]; + b = [f(); N]; + [f(); N]; // on arrays with > 0 repeat let a = [f(); 1]; @@ -58,3 +58,15 @@ fn main() { // as function param drop(vec![f(); 1]); } + +macro_rules! LEN { + () => { + 0 + }; +} + +fn issue_13110() { + let _data = [f(); LEN!()]; + const LENGTH: usize = LEN!(); + let _data = [f(); LENGTH]; +} diff --git a/tests/ui/zero_repeat_side_effects.rs b/tests/ui/zero_repeat_side_effects.rs index 9d9c36737..ed5cdd1ae 100644 --- a/tests/ui/zero_repeat_side_effects.rs +++ b/tests/ui/zero_repeat_side_effects.rs @@ -16,10 +16,8 @@ fn main() { // on arrays let a = [f(); 0]; - let a = [f(); N]; let mut b; b = [f(); 0]; - b = [f(); N]; // on vecs // vecs dont support infering value of consts @@ -39,9 +37,11 @@ fn main() { // when singled out/not part of assignment/local vec![f(); 0]; [f(); 0]; - [f(); N]; // should not trigger + let a = [f(); N]; + b = [f(); N]; + [f(); N]; // on arrays with > 0 repeat let a = [f(); 1]; @@ -58,3 +58,15 @@ fn main() { // as function param drop(vec![f(); 1]); } + +macro_rules! LEN { + () => { + 0 + }; +} + +fn issue_13110() { + let _data = [f(); LEN!()]; + const LENGTH: usize = LEN!(); + let _data = [f(); LENGTH]; +} diff --git a/tests/ui/zero_repeat_side_effects.stderr b/tests/ui/zero_repeat_side_effects.stderr index afdc60542..d578e22b9 100644 --- a/tests/ui/zero_repeat_side_effects.stderr +++ b/tests/ui/zero_repeat_side_effects.stderr @@ -8,70 +8,52 @@ LL | let a = [f(); 0]; = help: to override `-D warnings` add `#[allow(clippy::zero_repeat_side_effects)]` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:19:5 - | -LL | let a = [f(); N]; - | ^^^^^^^^^^^^^^^^^ help: consider using: `f(); let a: [i32; 0] = [];` - -error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:21:5 + --> tests/ui/zero_repeat_side_effects.rs:20:5 | LL | b = [f(); 0]; | ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:22:5 - | -LL | b = [f(); N]; - | ^^^^^^^^^^^^ help: consider using: `f(); b = [] as [i32; 0]` - -error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:26:5 + --> tests/ui/zero_repeat_side_effects.rs:24:5 | LL | let c = vec![f(); 0]; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f(); let c: std::vec::Vec = vec![];` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:28:5 + --> tests/ui/zero_repeat_side_effects.rs:26:5 | LL | d = vec![f(); 0]; | ^^^^^^^^^^^^^^^^ help: consider using: `f(); d = vec![] as std::vec::Vec` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:31:5 + --> tests/ui/zero_repeat_side_effects.rs:29:5 | LL | let e = [println!("side effect"); 0]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `println!("side effect"); let e: [(); 0] = [];` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:34:5 + --> tests/ui/zero_repeat_side_effects.rs:32:5 | LL | let g = [{ f() }; 0]; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `{ f() }; let g: [i32; 0] = [];` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:37:10 + --> tests/ui/zero_repeat_side_effects.rs:35:10 | LL | drop(vec![f(); 0]); | ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec }` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:40:5 + --> tests/ui/zero_repeat_side_effects.rs:38:5 | LL | vec![f(); 0]; | ^^^^^^^^^^^^ help: consider using: `{ f(); vec![] as std::vec::Vec }` error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:41:5 + --> tests/ui/zero_repeat_side_effects.rs:39:5 | LL | [f(); 0]; | ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }` -error: function or method calls as the initial value in zero-sized array initializers may cause side effects - --> tests/ui/zero_repeat_side_effects.rs:42:5 - | -LL | [f(); N]; - | ^^^^^^^^ help: consider using: `{ f(); [] as [i32; 0] }` - -error: aborting due to 12 previous errors +error: aborting due to 9 previous errors