Skip to content

Commit

Permalink
Fix a FP in missing_const_for_fn
Browse files Browse the repository at this point in the history
where a function that calls a standard library function whose constness
is unstable is considered as being able to be a const function
  • Loading branch information
rail-rain committed Apr 13, 2021
1 parent e9728b8 commit 26a1989
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 14 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/missing_const_for_fn.rs
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/lib.rs
Expand Up @@ -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;
Expand Down
40 changes: 36 additions & 4 deletions 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::{
Expand All @@ -6,14 +11,15 @@ 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;
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 {
Expand Down Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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 { .. }
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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
}
}
8 changes: 8 additions & 0 deletions 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() {}
19 changes: 19 additions & 0 deletions tests/ui/missing_const_for_fn/cant_be_const.rs
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
}
10 changes: 10 additions & 0 deletions 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;

Expand Down Expand Up @@ -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() {}
26 changes: 17 additions & 9 deletions 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 }
Expand All @@ -9,23 +9,23 @@ 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
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
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;
Expand All @@ -34,36 +34,44 @@ 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()
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
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
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

0 comments on commit 26a1989

Please sign in to comment.