about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-04-14 04:39:25 +0000
committerbors <bors@rust-lang.org>2021-04-14 04:39:25 +0000
commit19740d9334d1f4260a2851c3db7a7e70eb3d2ec3 (patch)
treebfd246df1c05d96fd056accad0ea5e7ae9f18f72
parent8f3c2450c2dff0d4ae82395699809d3ea726d6c2 (diff)
parent26a1989041393116c001f17d0323d6b4fc989dd9 (diff)
downloadrust-19740d9334d1f4260a2851c3db7a7e70eb3d2ec3.tar.gz
rust-19740d9334d1f4260a2851c3db7a7e70eb3d2ec3.zip
Auto merge of #7076 - rail-rain:missing_const_for_fn, r=phansch
Fix a FP in `missing_const_for_fn`

where a function that calls a standard library function whose constness
is unstable is considered as being able to be a const function. Fixes #5995.

The core change is the move from `rustc_mir::const_eval::is_min_const_fn` to `rustc_mir::const_eval::is_const_fn`. I'm not clear about the difference in their purpose between them so I'm not sure if it's acceptable to call `qualify_min_const_fn::is_min_const_fn` this way now.

---

changelog: `missing_const_for_fn`: No longer lints when an unstably const function is called
-rw-r--r--clippy_lints/src/missing_const_for_fn.rs2
-rw-r--r--clippy_utils/src/lib.rs1
-rw-r--r--clippy_utils/src/qualify_min_const_fn.rs40
-rw-r--r--tests/ui/missing_const_for_fn/auxiliary/helper.rs8
-rw-r--r--tests/ui/missing_const_for_fn/cant_be_const.rs19
-rw-r--r--tests/ui/missing_const_for_fn/could_be_const.rs10
-rw-r--r--tests/ui/missing_const_for_fn/could_be_const.stderr26
7 files changed, 92 insertions, 14 deletions
diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs
index 0dc02431ad5..93b7a897405 100644
--- a/clippy_lints/src/missing_const_for_fn.rs
+++ b/clippy_lints/src/missing_const_for_fn.rs
@@ -138,7 +138,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) {
+        if let Err((span, err)) = is_min_const_fn(cx.tcx, mir, self.msrv.as_ref()) {
             if rustc_mir::const_eval::is_min_const_fn(cx.tcx, def_id.to_def_id()) {
                 cx.tcx.sess.span_err(span, &err);
             }
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index ec140463a8a..8e1a2105b96 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -9,6 +9,7 @@
 // (Currently there is no way to opt into sysroot crates without `extern crate`.)
 extern crate rustc_ast;
 extern crate rustc_ast_pretty;
+extern crate rustc_attr;
 extern crate rustc_data_structures;
 extern crate rustc_errors;
 extern crate rustc_hir;
diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs
index b52cbf31e35..b2ce58b597b 100644
--- a/clippy_utils/src/qualify_min_const_fn.rs
+++ b/clippy_utils/src/qualify_min_const_fn.rs
@@ -1,3 +1,8 @@
+// This code used to be a part of `rustc` but moved to Clippy as a result of
+// https://github.com/rust-lang/rust/issues/76618. Because of that, it contains unused code and some
+// of terminologies might not be relevant in the context of Clippy. Note that its behavior might
+// differ from the time of `rustc` even if the name stays the same.
+
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::{
@@ -6,6 +11,7 @@ use rustc_middle::mir::{
 };
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt};
+use rustc_semver::RustcVersion;
 use rustc_span::symbol::sym;
 use rustc_span::Span;
 use rustc_target::spec::abi::Abi::RustIntrinsic;
@@ -13,7 +19,7 @@ use std::borrow::Cow;
 
 type McfResult = Result<(), (Span, Cow<'static, str>)>;
 
-pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>) -> McfResult {
+pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, msrv: Option<&RustcVersion>) -> McfResult {
     let def_id = body.source.def_id();
     let mut current = def_id;
     loop {
@@ -70,7 +76,7 @@ pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>) -> McfResult {
     )?;
 
     for bb in body.basic_blocks() {
-        check_terminator(tcx, body, bb.terminator())?;
+        check_terminator(tcx, body, bb.terminator(), msrv)?;
         for stmt in &bb.statements {
             check_statement(tcx, body, def_id, stmt)?;
         }
@@ -268,7 +274,12 @@ fn check_place(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'t
     Ok(())
 }
 
-fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Terminator<'tcx>) -> McfResult {
+fn check_terminator(
+    tcx: TyCtxt<'tcx>,
+    body: &'a Body<'tcx>,
+    terminator: &Terminator<'tcx>,
+    msrv: Option<&RustcVersion>,
+) -> McfResult {
     let span = terminator.source_info.span;
     match &terminator.kind {
         TerminatorKind::FalseEdge { .. }
@@ -305,7 +316,7 @@ fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Termin
         } => {
             let fn_ty = func.ty(body, tcx);
             if let ty::FnDef(fn_def_id, _) = *fn_ty.kind() {
-                if !rustc_mir::const_eval::is_min_const_fn(tcx, fn_def_id) {
+                if !is_const_fn(tcx, fn_def_id, msrv) {
                     return Err((
                         span,
                         format!(
@@ -350,3 +361,24 @@ fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Termin
         TerminatorKind::InlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())),
     }
 }
+
+fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: Option<&RustcVersion>) -> bool {
+    rustc_mir::const_eval::is_const_fn(tcx, def_id)
+        && if let Some(const_stab) = tcx.lookup_const_stability(def_id) {
+            if let rustc_attr::StabilityLevel::Stable { since } = const_stab.level {
+                // Checking MSRV is manually necessary because `rustc` has no such concept. This entire
+                // function could be removed if `rustc` provided a MSRV-aware version of `is_const_fn`.
+                // as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262.
+                crate::meets_msrv(
+                    msrv,
+                    &RustcVersion::parse(&since.as_str())
+                        .expect("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted"),
+                )
+            } else {
+                // `rustc_mir::const_eval::is_const_fn` should return false for unstably const functions.
+                unreachable!();
+            }
+        } else {
+            true
+        }
+}
diff --git a/tests/ui/missing_const_for_fn/auxiliary/helper.rs b/tests/ui/missing_const_for_fn/auxiliary/helper.rs
new file mode 100644
index 00000000000..7b9dc76b8f1
--- /dev/null
+++ b/tests/ui/missing_const_for_fn/auxiliary/helper.rs
@@ -0,0 +1,8 @@
+// This file provides a const function that is unstably const forever.
+
+#![feature(staged_api)]
+#![stable(feature = "1", since = "1.0.0")]
+
+#[stable(feature = "1", since = "1.0.0")]
+#[rustc_const_unstable(feature = "foo", issue = "none")]
+pub const fn unstably_const_fn() {}
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 ba352ef9ee9..7cda1aaa3c2 100644
--- a/tests/ui/missing_const_for_fn/cant_be_const.rs
+++ b/tests/ui/missing_const_for_fn/cant_be_const.rs
@@ -2,9 +2,14 @@
 //! compilation error.
 //! The .stderr output of this test should be empty. Otherwise it's a bug somewhere.
 
+// aux-build:helper.rs
+
 #![warn(clippy::missing_const_for_fn)]
 #![allow(incomplete_features)]
 #![feature(start, const_generics)]
+#![feature(custom_inner_attributes)]
+
+extern crate helper;
 
 struct Game;
 
@@ -101,3 +106,17 @@ fn const_generic_return<T, const N: usize>(t: &[T]) -> &[T; N] {
 
     unsafe { &*p }
 }
+
+// Do not lint this because it calls a function whose constness is unstable.
+fn unstably_const_fn() {
+    helper::unstably_const_fn()
+}
+
+mod const_fn_stabilized_after_msrv {
+    #![clippy::msrv = "1.46.0"]
+
+    // Do not lint this because `u8::is_ascii_digit` is stabilized as a const function in 1.47.0.
+    fn const_fn_stabilized_after_msrv(byte: u8) {
+        byte.is_ascii_digit();
+    }
+}
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 c6f44b7daa3..0accb516f5f 100644
--- a/tests/ui/missing_const_for_fn/could_be_const.rs
+++ b/tests/ui/missing_const_for_fn/could_be_const.rs
@@ -1,6 +1,7 @@
 #![warn(clippy::missing_const_for_fn)]
 #![allow(incomplete_features, clippy::let_and_return)]
 #![feature(const_generics)]
+#![feature(custom_inner_attributes)]
 
 use std::mem::transmute;
 
@@ -70,5 +71,14 @@ mod with_drop {
     }
 }
 
+mod const_fn_stabilized_before_msrv {
+    #![clippy::msrv = "1.47.0"]
+
+    // This could be const because `u8::is_ascii_digit` is a stable const function in 1.47.
+    fn const_fn_stabilized_before_msrv(byte: u8) {
+        byte.is_ascii_digit();
+    }
+}
+
 // Should not be const
 fn main() {}
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 74d32b8a1aa..63c211f39fa 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:13: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:17: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:23: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:28: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:34: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:39: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:44:1
+  --> $DIR/could_be_const.rs:45:1
    |
 LL | / fn generic<T>(t: T) -> T {
 LL | |     t
@@ -58,12 +58,20 @@ LL | | }
    | |_^
 
 error: this could be a `const fn`
-  --> $DIR/could_be_const.rs:67:9
+  --> $DIR/could_be_const.rs:68:9
    |
 LL | /         pub fn b(self, a: &A) -> B {
 LL | |             B
 LL | |         }
    | |_________^
 
-error: aborting due to 8 previous errors
+error: this could be a `const fn`
+  --> $DIR/could_be_const.rs:78:5
+   |
+LL | /     fn const_fn_stabilized_before_msrv(byte: u8) {
+LL | |         byte.is_ascii_digit();
+LL | |     }
+   | |_____^
+
+error: aborting due to 9 previous errors