From 6fdf295664acc28ebb018e1d7b97b9e9e1e8f0ea Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 16 Apr 2024 20:25:35 +0200 Subject: [PATCH 1/3] let `qualify_min_const_fn` deal with drop checks --- clippy_lints/src/missing_const_for_fn.rs | 15 +-------------- .../ui/missing_const_for_fn/could_be_const.rs | 18 ++++++++++++++++++ .../missing_const_for_fn/could_be_const.stderr | 18 +++++++++++++++++- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 9ba19e0a86581..fe636f27a0f07 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -1,7 +1,6 @@ use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint; use clippy_utils::qualify_min_const_fn::is_min_const_fn; -use clippy_utils::ty::has_drop; use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, is_from_proc_macro, trait_ref_of_method}; use rustc_hir as hir; use rustc_hir::def_id::CRATE_DEF_ID; @@ -121,10 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { } }, FnKind::Method(_, sig, ..) => { - if trait_ref_of_method(cx, def_id).is_some() - || already_const(sig.header) - || method_accepts_droppable(cx, def_id) - { + if trait_ref_of_method(cx, def_id).is_some() || already_const(sig.header) { return; } }, @@ -162,15 +158,6 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { extract_msrv_attr!(LateContext); } -/// Returns true if any of the method parameters is a type that implements `Drop`. The method -/// can't be made const then, because `drop` can't be const-evaluated. -fn method_accepts_droppable(cx: &LateContext<'_>, def_id: LocalDefId) -> bool { - let sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder(); - - // If any of the params are droppable, return true - sig.inputs().iter().any(|&ty| has_drop(cx, ty)) -} - // We don't have to lint on something that's already `const` #[must_use] fn already_const(header: hir::FnHeader) -> bool { diff --git a/tests/ui/missing_const_for_fn/could_be_const.rs b/tests/ui/missing_const_for_fn/could_be_const.rs index 06dbbeb31c0d5..8e0ede478b56c 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.rs +++ b/tests/ui/missing_const_for_fn/could_be_const.rs @@ -141,3 +141,21 @@ mod msrv { let _ = unsafe { bar.val }; } } + +mod issue12677 { + pub struct Wrapper { + pub strings: Vec, + } + + impl Wrapper { + #[must_use] + pub fn new(strings: Vec) -> Self { + Self { strings } + } + + #[must_use] + pub fn empty() -> Self { + Self { strings: Vec::new() } + } + } +} diff --git a/tests/ui/missing_const_for_fn/could_be_const.stderr b/tests/ui/missing_const_for_fn/could_be_const.stderr index b2cade3056373..84ad6357b7bcf 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.stderr +++ b/tests/ui/missing_const_for_fn/could_be_const.stderr @@ -130,5 +130,21 @@ LL | | let _ = unsafe { bar.val }; LL | | } | |_____^ -error: aborting due to 14 previous errors +error: this could be a `const fn` + --> tests/ui/missing_const_for_fn/could_be_const.rs:152:9 + | +LL | / pub fn new(strings: Vec) -> Self { +LL | | Self { strings } +LL | | } + | |_________^ + +error: this could be a `const fn` + --> tests/ui/missing_const_for_fn/could_be_const.rs:157:9 + | +LL | / pub fn empty() -> Self { +LL | | Self { strings: Vec::new() } +LL | | } + | |_________^ + +error: aborting due to 16 previous errors From 973f318514cec40bd388f9e6f1acacc9e2bed436 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 16 Apr 2024 21:27:34 +0200 Subject: [PATCH 2/3] qualify_min_const_fn: ignore cleanup bbs --- clippy_utils/src/qualify_min_const_fn.rs | 10 +++++++--- tests/ui/missing_const_for_fn/could_be_const.rs | 12 ++++++++++++ tests/ui/missing_const_for_fn/could_be_const.stderr | 11 ++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index 81e94725a70cb..42b10f69c0cdf 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -40,9 +40,13 @@ pub fn is_min_const_fn<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, msrv: &Msrv) )?; for bb in &*body.basic_blocks { - check_terminator(tcx, body, bb.terminator(), msrv)?; - for stmt in &bb.statements { - check_statement(tcx, body, def_id, stmt, msrv)?; + // Cleanup blocks are ignored entirely by const eval, so we can too: + // https://github.com/rust-lang/rust/blob/1dea922ea6e74f99a0e97de5cdb8174e4dea0444/compiler/rustc_const_eval/src/transform/check_consts/check.rs#L382 + if !bb.is_cleanup { + check_terminator(tcx, body, bb.terminator(), msrv)?; + for stmt in &bb.statements { + check_statement(tcx, body, def_id, stmt, msrv)?; + } } } Ok(()) diff --git a/tests/ui/missing_const_for_fn/could_be_const.rs b/tests/ui/missing_const_for_fn/could_be_const.rs index 8e0ede478b56c..58e639cc7fd1f 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.rs +++ b/tests/ui/missing_const_for_fn/could_be_const.rs @@ -158,4 +158,16 @@ mod issue12677 { Self { strings: Vec::new() } } } + + pub struct Other { + pub text: String, + pub vec: Vec, + } + + impl Other { + pub fn new(text: String) -> Self { + let vec = Vec::new(); + Self { text, vec } + } + } } diff --git a/tests/ui/missing_const_for_fn/could_be_const.stderr b/tests/ui/missing_const_for_fn/could_be_const.stderr index 84ad6357b7bcf..1c61c3e871324 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.stderr +++ b/tests/ui/missing_const_for_fn/could_be_const.stderr @@ -146,5 +146,14 @@ LL | | Self { strings: Vec::new() } LL | | } | |_________^ -error: aborting due to 16 previous errors +error: this could be a `const fn` + --> tests/ui/missing_const_for_fn/could_be_const.rs:168:9 + | +LL | / pub fn new(text: String) -> Self { +LL | | let vec = Vec::new(); +LL | | Self { text, vec } +LL | | } + | |_________^ + +error: aborting due to 17 previous errors From c3d3a3f30164644d64c09812c863415f28604e80 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:54:33 +0200 Subject: [PATCH 3/3] remove unnecessary const error handling in `missing_const_for_fn` --- clippy_lints/src/missing_const_for_fn.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index fe636f27a0f07..4592324809fa2 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -147,11 +147,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { let mir = cx.tcx.optimized_mir(def_id); - if let Err((span, err)) = is_min_const_fn(cx.tcx, mir, &self.msrv) { - if cx.tcx.is_const_fn_raw(def_id.to_def_id()) { - cx.tcx.dcx().span_err(span, err); - } - } else { + if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) { span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`"); } }