diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 2003dd1fb..ed4cdce8c 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -1,7 +1,8 @@ -use clippy_utils::consts::{constant_with_source, Constant, ConstantSource}; +use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_inside_always_const_context; use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn}; -use rustc_hir::{Expr, Item, ItemKind, Node}; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -42,17 +43,16 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants { let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else { return; }; - let Some((Constant::Bool(val), source)) = constant_with_source(cx, cx.typeck_results(), condition) else { + let Some(Constant::Bool(val)) = constant(cx, cx.typeck_results(), condition) else { return; }; - if let ConstantSource::Constant = source - && let Node::Item(Item { - kind: ItemKind::Const(..), - .. - }) = cx.tcx.parent_hir_node(e.hir_id) - { - return; + + match condition.kind { + ExprKind::Path(..) | ExprKind::Lit(_) => {}, + _ if is_inside_always_const_context(cx.tcx, e.hir_id) => return, + _ => {}, } + if val { span_lint_and_help( cx, diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index ff631909b..9af73db68 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -50,6 +50,8 @@ declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]); impl<'tcx> LateLintPass<'tcx> for DefaultNumericFallback { fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'tcx>) { let hir = cx.tcx.hir(); + // NOTE: this is different from `clippy_utils::is_inside_always_const_context`. + // Inline const supports type inference. let is_parent_const = matches!( hir.body_const_context(hir.body_owner_def_id(body.id())), Some(ConstContext::Const { inline: false } | ConstContext::Static(_)) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 87f886b11..139c33d3f 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::has_drop; -use clippy_utils::{any_parent_is_automatically_derived, is_lint_allowed, path_to_local, peel_blocks}; +use clippy_utils::{ + any_parent_is_automatically_derived, is_inside_always_const_context, is_lint_allowed, path_to_local, peel_blocks, +}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{ @@ -258,13 +260,16 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) { if let StmtKind::Semi(expr) = stmt.kind + && !in_external_macro(cx.sess(), stmt.span) && let ctxt = stmt.span.ctxt() && expr.span.ctxt() == ctxt && let Some(reduced) = reduce_expression(cx, expr) - && !in_external_macro(cx.sess(), stmt.span) && reduced.iter().all(|e| e.span.ctxt() == ctxt) { if let ExprKind::Index(..) = &expr.kind { + if is_inside_always_const_context(cx.tcx, expr.hir_id) { + return; + } let snippet = if let (Some(arr), Some(func)) = (snippet_opt(cx, reduced[0].span), snippet_opt(cx, reduced[1].span)) { format!("assert!({}.len() > {});", &arr, &func) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 73de8aed3..8a6750c89 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -101,10 +101,10 @@ use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::{ - self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, Destination, Expr, - ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item, - ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, PathSegment, - PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, + self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, ConstContext, + Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, + ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, + PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, Level, Lint, LintContext}; @@ -209,7 +209,10 @@ pub fn local_is_initialized(cx: &LateContext<'_>, local: HirId) -> bool { false } -/// Returns `true` if the given `NodeId` is inside a constant context +/// Returns `true` if the given `HirId` is inside a constant context. +/// +/// This is the same as `is_inside_always_const_context`, but also includes +/// `const fn`. /// /// # Example /// @@ -222,6 +225,24 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool { cx.tcx.hir().is_inside_const_context(id) } +/// Returns `true` if the given `HirId` is inside an always constant context. +/// +/// This context includes: +/// * const/static items +/// * const blocks (or inline consts) +/// * associated constants +pub fn is_inside_always_const_context(tcx: TyCtxt<'_>, hir_id: HirId) -> bool { + use ConstContext::{Const, ConstFn, Static}; + let hir = tcx.hir(); + let Some(ctx) = hir.body_const_context(hir.enclosing_body_owner(hir_id)) else { + return false; + }; + match ctx { + ConstFn => false, + Static(_) | Const { inline: _ } => true, + } +} + /// Checks if a `Res` refers to a constructor of a `LangItem` /// For example, use this to check whether a function call or a pattern is `Some(..)`. pub fn is_res_lang_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool { diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index 1309ae45d..957154e60 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -1,4 +1,4 @@ -#![allow(non_fmt_panics, clippy::needless_bool)] +#![allow(non_fmt_panics, clippy::needless_bool, clippy::eq_op)] macro_rules! assert_const { ($len:expr) => { @@ -49,7 +49,16 @@ fn main() { const _: () = assert!(true); //~^ ERROR: `assert!(true)` will be optimized out by the compiler + assert!(8 == (7 + 1)); + //~^ ERROR: `assert!(true)` will be optimized out by the compiler + // Don't lint if the value is dependent on a defined constant: const N: usize = 1024; const _: () = assert!(N.is_power_of_two()); } + +const _: () = { + assert!(true); + //~^ ERROR: `assert!(true)` will be optimized out by the compiler + assert!(8 == (7 + 1)); +}; diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr index 00f117c94..e164a999c 100644 --- a/tests/ui/assertions_on_constants.stderr +++ b/tests/ui/assertions_on_constants.stderr @@ -80,5 +80,21 @@ LL | const _: () = assert!(true); | = help: remove it -error: aborting due to 10 previous errors +error: `assert!(true)` will be optimized out by the compiler + --> tests/ui/assertions_on_constants.rs:52:5 + | +LL | assert!(8 == (7 + 1)); + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: remove it + +error: `assert!(true)` will be optimized out by the compiler + --> tests/ui/assertions_on_constants.rs:61:5 + | +LL | assert!(true); + | ^^^^^^^^^^^^^ + | + = help: remove it + +error: aborting due to 12 previous errors diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed index 11761c6c9..006f123cb 100644 --- a/tests/ui/unnecessary_operation.fixed +++ b/tests/ui/unnecessary_operation.fixed @@ -43,7 +43,7 @@ fn get_number() -> i32 { 0 } -fn get_usize() -> usize { +const fn get_usize() -> usize { 0 } fn get_struct() -> Struct { @@ -113,4 +113,16 @@ fn main() { 'label: { break 'label }; + let () = const { + [42, 55][get_usize()]; + }; +} + +const _: () = { + [42, 55][get_usize()]; +}; + +const fn foo() { + assert!([42, 55].len() > get_usize()); + //~^ ERROR: unnecessary operation } diff --git a/tests/ui/unnecessary_operation.rs b/tests/ui/unnecessary_operation.rs index de0081289..b4067c740 100644 --- a/tests/ui/unnecessary_operation.rs +++ b/tests/ui/unnecessary_operation.rs @@ -43,7 +43,7 @@ fn get_number() -> i32 { 0 } -fn get_usize() -> usize { +const fn get_usize() -> usize { 0 } fn get_struct() -> Struct { @@ -117,4 +117,16 @@ fn main() { 'label: { break 'label }; + let () = const { + [42, 55][get_usize()]; + }; +} + +const _: () = { + [42, 55][get_usize()]; +}; + +const fn foo() { + [42, 55][get_usize()]; + //~^ ERROR: unnecessary operation } diff --git a/tests/ui/unnecessary_operation.stderr b/tests/ui/unnecessary_operation.stderr index 27be5e6f4..036a9a44b 100644 --- a/tests/ui/unnecessary_operation.stderr +++ b/tests/ui/unnecessary_operation.stderr @@ -119,5 +119,11 @@ LL | | s: String::from("blah"), LL | | }; | |______^ help: statement can be reduced to: `String::from("blah");` -error: aborting due to 19 previous errors +error: unnecessary operation + --> tests/ui/unnecessary_operation.rs:130:5 + | +LL | [42, 55][get_usize()]; + | ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());` + +error: aborting due to 20 previous errors