diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index e600dc58c9da..311f6dbc696d 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -20,8 +20,8 @@ extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; -extern crate rustc_data_structures; extern crate rustc_const_eval; +extern crate rustc_data_structures; // The `rustc_driver` crate seems to be required in order to use the `rust_ast` crate. #[allow(unused_extern_crates)] extern crate rustc_driver; diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index cdaf8985d5e7..799b6ffeba6b 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -4,7 +4,7 @@ // differ from the time of `rustc` even if the name stays the same. use crate::msrvs::Msrv; -use hir::{Constness, LangItem}; +use hir::LangItem; use rustc_const_eval::transform::check_consts::ConstCx; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -14,14 +14,14 @@ use rustc_middle::mir::{ Body, CastKind, NonDivergingIntrinsic, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; -use rustc_middle::traits::ObligationCause; +use rustc_middle::traits::{ImplSource, ObligationCause}; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt}; use rustc_middle::ty::{BoundConstness, TraitRef}; use rustc_semver::RustcVersion; use rustc_span::symbol::sym; use rustc_span::Span; -use rustc_trait_selection::traits::SelectionContext; +use rustc_trait_selection::traits::{ObligationCtxt, SelectionContext}; use std::borrow::Cow; type McfResult = Result<(), (Span, Cow<'static, str>)>; @@ -135,9 +135,9 @@ fn check_rvalue<'tcx>( match rvalue { Rvalue::ThreadLocalRef(_) => Err((span, "cannot access thread local storage in const fn".into())), Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => { - check_place(tcx, *place, span, body) + check_place(tcx, *place, span, body, false) }, - Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body), + Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body, false), Rvalue::Repeat(operand, _) | Rvalue::Use(operand) | Rvalue::Cast( @@ -230,14 +230,14 @@ fn check_statement<'tcx>( let span = statement.source_info.span; match &statement.kind { StatementKind::Assign(box (place, rval)) => { - check_place(tcx, *place, span, body)?; + check_place(tcx, *place, span, body, false)?; check_rvalue(tcx, body, def_id, rval, span) }, - StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body), + StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body, false), // just an assignment StatementKind::SetDiscriminant { place, .. } | StatementKind::Deinit(place) => { - check_place(tcx, **place, span, body) + check_place(tcx, **place, span, body, false) }, StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => check_operand(tcx, op, span, body), @@ -263,7 +263,8 @@ fn check_statement<'tcx>( fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult { match operand { - Operand::Move(place) | Operand::Copy(place) => check_place(tcx, *place, span, body), + Operand::Move(place) => check_place(tcx, *place, span, body, true), + Operand::Copy(place) => check_place(tcx, *place, span, body, false), Operand::Constant(c) => match c.check_static_ptr(tcx) { Some(_) => Err((span, "cannot access `static` items in const fn".into())), None => Ok(()), @@ -271,13 +272,20 @@ fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, b } } -fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult { +fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'tcx>, in_move: bool) -> McfResult { let mut cursor = place.projection.as_ref(); while let [ref proj_base @ .., elem] = *cursor { cursor = proj_base; match elem { - ProjectionElem::Field(..) => { + ProjectionElem::Field(_, ty) => { + if !is_ty_const_destruct(tcx, ty, body) && in_move { + return Err(( + span, + "cannot drop locals with a non constant destructor in const fn".into(), + )); + } + let base_ty = Place::ty_from(place.local, proj_base, body, tcx).ty; if let Some(def) = base_ty.ty_adt_def() { // No union field accesses in `const fn` @@ -320,7 +328,7 @@ fn check_terminator<'tcx>( "cannot drop locals with a non constant destructor in const fn".into(), )); } - check_place(tcx, *place, span, body) + check_place(tcx, *place, span, body, false) }, TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body), TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { @@ -408,6 +416,7 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { }) } +#[expect(clippy::similar_names)] // bit too pedantic fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx>) -> bool { // Avoid selecting for simple cases, such as builtin types. if ty::util::is_trivially_const_drop(ty) { @@ -417,23 +426,24 @@ fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx> let obligation = Obligation::new( tcx, ObligationCause::dummy_with_span(body.span), - ConstCx::new(tcx, body).param_env.with_constness(Constness::Const), + ConstCx::new(tcx, body).param_env.with_const(), TraitRef::from_lang_item(tcx, LangItem::Destruct, body.span, [ty]).with_constness(BoundConstness::ConstIfConst), ); - let fields_all_const_destruct = if let ty::Adt(def, subst) = ty.kind() && !ty.is_union() { - // This is such a mess even rustfmt doesn't wanna touch it - def.all_fields() - .map(|field| is_ty_const_destruct(tcx, field.ty(tcx, subst), body)) - .all(|f| f) - && def.variants().iter() - .map(|variant| variant.fields.iter().map(|field| is_ty_const_destruct(tcx, field.ty(tcx, subst), body))) - .all(|mut fs| fs.all(|f| f)) - } else { - true - }; - let infcx = tcx.infer_ctxt().build(); let mut selcx = SelectionContext::new(&infcx); - selcx.select(&obligation).is_ok() && fields_all_const_destruct + let Some(impl_src) = selcx.select(&obligation).ok().flatten() else { + return false; + }; + + if !matches!( + impl_src, + ImplSource::ConstDestruct(_) | ImplSource::Param(_, ty::BoundConstness::ConstIfConst) + ) { + return false; + } + + let ocx = ObligationCtxt::new(&infcx); + ocx.register_obligations(impl_src.nested_obligations()); + ocx.select_all_or_error().is_empty() } diff --git a/tests/ui/missing_const_for_fn/cant_be_const.rs b/tests/ui/missing_const_for_fn/cant_be_const.rs index 4fe02a9cef87..286d208b25bd 100644 --- a/tests/ui/missing_const_for_fn/cant_be_const.rs +++ b/tests/ui/missing_const_for_fn/cant_be_const.rs @@ -127,6 +127,7 @@ with_span! { fn dont_check_in_proc_macro() {} } +// Do not lint `String` has `Vec`, which cannot be dropped in const contexts fn a(this: String) {} enum A { @@ -134,6 +135,25 @@ enum A { N, } +// Same here. fn b(this: A) {} +// Minimized version of `a`. fn c(this: Vec) {} + +struct F(A); + +// Do not lint +fn f(this: F) {} + +// Do not lint +fn g(this: T) {} + +struct Issue10617(String); + +impl Issue10617 { + // Do not lint + pub fn name(self) -> String { + self.0 + } +} 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 0246c8622ed3..b1980b1b523f 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.rs +++ b/tests/ui/missing_const_for_fn/could_be_const.rs @@ -1,5 +1,7 @@ #![warn(clippy::missing_const_for_fn)] #![allow(incomplete_features, clippy::let_and_return)] +#![feature(const_mut_refs)] +#![feature(const_trait_impl)] use std::mem::transmute; @@ -87,3 +89,14 @@ fn msrv_1_46() -> i32 { // Should not be const fn main() {} + +struct D; + +impl const Drop for D { + fn drop(&mut self) { + todo!(); + } +} + +// Lint this, since it can be dropped in const contexts +fn d(this: D) {} 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 955e1ed26340..7be2cc0ca930 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.stderr +++ b/tests/ui/missing_const_for_fn/could_be_const.stderr @@ -1,5 +1,5 @@ error: this could be a `const fn` - --> $DIR/could_be_const.rs:12:5 + --> $DIR/could_be_const.rs:14:5 | LL | / pub fn new() -> Self { LL | | Self { guess: 42 } @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::missing-const-for-fn` implied by `-D warnings` error: this could be a `const fn` - --> $DIR/could_be_const.rs:16:5 + --> $DIR/could_be_const.rs:18:5 | LL | / fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] { LL | | b @@ -17,7 +17,7 @@ LL | | } | |_____^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:22:1 + --> $DIR/could_be_const.rs:24:1 | LL | / fn one() -> i32 { LL | | 1 @@ -25,7 +25,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:27:1 + --> $DIR/could_be_const.rs:29:1 | LL | / fn two() -> i32 { LL | | let abc = 2; @@ -34,7 +34,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:33:1 + --> $DIR/could_be_const.rs:35:1 | LL | / fn string() -> String { LL | | String::new() @@ -42,7 +42,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:38:1 + --> $DIR/could_be_const.rs:40:1 | LL | / unsafe fn four() -> i32 { LL | | 4 @@ -50,7 +50,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:43:1 + --> $DIR/could_be_const.rs:45:1 | LL | / fn generic(t: T) -> T { LL | | t @@ -58,7 +58,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:51:1 + --> $DIR/could_be_const.rs:53:1 | LL | / fn generic_arr(t: [T; 1]) -> T { LL | | t[0] @@ -66,7 +66,7 @@ LL | | } | |_^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:64:9 + --> $DIR/could_be_const.rs:66:9 | LL | / pub fn b(self, a: &A) -> B { LL | | B @@ -74,7 +74,7 @@ LL | | } | |_________^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:73:5 + --> $DIR/could_be_const.rs:75:5 | LL | / fn const_fn_stabilized_before_msrv(byte: u8) { LL | | byte.is_ascii_digit(); @@ -82,12 +82,18 @@ LL | | } | |_____^ error: this could be a `const fn` - --> $DIR/could_be_const.rs:84:1 + --> $DIR/could_be_const.rs:86:1 | LL | / fn msrv_1_46() -> i32 { LL | | 46 LL | | } | |_^ -error: aborting due to 11 previous errors +error: this could be a `const fn` + --> $DIR/could_be_const.rs:102:1 + | +LL | fn d(this: D) {} + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors