about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-06-13 18:37:27 +0000
committerbors <bors@rust-lang.org>2023-06-13 18:37:27 +0000
commiteefc2a0ac4e72d5532f9e0d9bd76971bdac3f597 (patch)
tree509494cdc2d66d5a1bc9f3527af2b9138b79b6dd
parent72332b2598b3077ef6241eadcb807d1f89175af0 (diff)
parent9bc5a146fdfb521034eb7b9b907e1d113791d30d (diff)
downloadrust-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.rs1
-rw-r--r--clippy_utils/src/qualify_min_const_fn.rs73
-rw-r--r--tests/ui/missing_const_for_fn/cant_be_const.rs33
-rw-r--r--tests/ui/missing_const_for_fn/could_be_const.rs13
-rw-r--r--tests/ui/missing_const_for_fn/could_be_const.stderr30
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