Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a FP in missing_const_for_fn #7076

Merged
merged 1 commit into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/missing_const_for_fn.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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