diff options
| author | bors <bors@rust-lang.org> | 2023-06-13 18:37:27 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-06-13 18:37:27 +0000 |
| commit | eefc2a0ac4e72d5532f9e0d9bd76971bdac3f597 (patch) | |
| tree | 509494cdc2d66d5a1bc9f3527af2b9138b79b6dd | |
| parent | 72332b2598b3077ef6241eadcb807d1f89175af0 (diff) | |
| parent | 9bc5a146fdfb521034eb7b9b907e1d113791d30d (diff) | |
| download | rust-eefc2a0ac4e72d5532f9e0d9bd76971bdac3f597.tar.gz rust-eefc2a0ac4e72d5532f9e0d9bd76971bdac3f597.zip | |
Auto merge of #10891 - Centri3:missing_const_for_fn, r=Jarcho
[`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct` this will check every local for `TerminatorKind::Drop` to ensure they can be evaluated at compile time, not sure if this is the best way to do this but MIR is confusing and it works so... fixes #10617 changelog: [`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
| -rw-r--r-- | clippy_utils/src/lib.rs | 1 | ||||
| -rw-r--r-- | clippy_utils/src/qualify_min_const_fn.rs | 73 | ||||
| -rw-r--r-- | tests/ui/missing_const_for_fn/cant_be_const.rs | 33 | ||||
| -rw-r--r-- | tests/ui/missing_const_for_fn/could_be_const.rs | 13 | ||||
| -rw-r--r-- | tests/ui/missing_const_for_fn/could_be_const.stderr | 30 |
5 files changed, 127 insertions, 23 deletions
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8c883445a79..311f6dbc696 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -20,6 +20,7 @@ extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; +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)] diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index 67369288b70..dcd372f689d 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -4,17 +4,24 @@ // differ from the time of `rustc` even if the name stays the same. use crate::msrvs::Msrv; +use hir::LangItem; +use rustc_const_eval::transform::check_consts::ConstCx; use rustc_hir as hir; use rustc_hir::def_id::DefId; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::Obligation; use rustc_middle::mir::{ Body, CastKind, NonDivergingIntrinsic, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; +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::{ObligationCtxt, SelectionContext}; use std::borrow::Cow; type McfResult = Result<(), (Span, Cow<'static, str>)>; @@ -256,7 +263,19 @@ 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) => { + if !place.projection.as_ref().is_empty() + && !is_ty_const_destruct(tcx, place.ty(&body.local_decls, tcx).ty, body) + { + return Err(( + span, + "cannot drop locals with a non constant destructor in const fn".into(), + )); + } + + check_place(tcx, *place, span, body) + }, + Operand::Copy(place) => check_place(tcx, *place, span, body), Operand::Constant(c) => match c.check_static_ptr(tcx) { Some(_) => Err((span, "cannot access `static` items in const fn".into())), None => Ok(()), @@ -266,6 +285,7 @@ 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 { let mut cursor = place.projection.as_ref(); + while let [ref proj_base @ .., elem] = *cursor { cursor = proj_base; match elem { @@ -305,15 +325,19 @@ fn check_terminator<'tcx>( | TerminatorKind::Resume | TerminatorKind::Terminate | TerminatorKind::Unreachable => Ok(()), - - TerminatorKind::Drop { place, .. } => check_place(tcx, *place, span, body), - + TerminatorKind::Drop { place, .. } => { + if !is_ty_const_destruct(tcx, place.ty(&body.local_decls, tcx).ty, body) { + return Err(( + span, + "cannot drop locals with a non constant destructor in const fn".into(), + )); + } + Ok(()) + }, TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body), - TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { Err((span, "const fn generators are unstable".into())) }, - TerminatorKind::Call { func, args, @@ -357,7 +381,6 @@ fn check_terminator<'tcx>( Err((span, "can only call other const fns within const fn".into())) } }, - TerminatorKind::Assert { cond, expected: _, @@ -365,7 +388,6 @@ fn check_terminator<'tcx>( target: _, unwind: _, } => check_operand(tcx, cond, span, body), - TerminatorKind::InlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), } } @@ -379,8 +401,7 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { // as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262. // HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev. `rustc-semver` - // doesn't accept the `-dev` version number so we have to strip it - // off. + // doesn't accept the `-dev` version number so we have to strip it off. let short_version = since .as_str() .split('-') @@ -398,3 +419,35 @@ 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) { + return true; + } + + let obligation = Obligation::new( + tcx, + ObligationCause::dummy_with_span(body.span), + ConstCx::new(tcx, body).param_env.with_const(), + TraitRef::from_lang_item(tcx, LangItem::Destruct, body.span, [ty]).with_constness(BoundConstness::ConstIfConst), + ); + + let infcx = tcx.infer_ctxt().build(); + let mut selcx = SelectionContext::new(&infcx); + 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 5db73a7b8ea..286d208b25b 100644 --- a/tests/ui/missing_const_for_fn/cant_be_const.rs +++ b/tests/ui/missing_const_for_fn/cant_be_const.rs @@ -13,7 +13,7 @@ extern crate proc_macros; use proc_macros::with_span; -struct Game; +struct Game; // You just lost. // This should not be linted because it's already const const fn already_const() -> i32 { @@ -126,3 +126,34 @@ with_span! { span fn dont_check_in_proc_macro() {} } + +// Do not lint `String` has `Vec<u8>`, which cannot be dropped in const contexts +fn a(this: String) {} + +enum A { + F(String), + N, +} + +// Same here. +fn b(this: A) {} + +// Minimized version of `a`. +fn c(this: Vec<u16>) {} + +struct F(A); + +// Do not lint +fn f(this: F) {} + +// Do not lint +fn g<T>(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 0246c8622ed..b1980b1b523 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 955e1ed2634..7be2cc0ca93 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) -> 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: Copy>(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 |
