Skip to content

Commit a0ea5a1

Browse files
committed
Auto merge of #3648 - phansch:const_fn_lint, r=<try>
Add initial version of const_fn lint This adds an initial version of a lint that can tell if a function could be `const`. TODO: - [x] Finish up the docs - [ ] Fix the ICE cc #2440
2 parents 91ef4e3 + 4450951 commit a0ea5a1

File tree

9 files changed

+278
-1
lines changed

9 files changed

+278
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,7 @@ All notable changes to this project will be documented in this file.
756756
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
757757
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
758758
[`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
759+
[`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn
759760
[`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
760761
[`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
761762
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
99

10-
[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 292 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1111

1212
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1313

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ extern crate rustc_data_structures;
2424
#[allow(unused_extern_crates)]
2525
extern crate rustc_errors;
2626
#[allow(unused_extern_crates)]
27+
extern crate rustc_mir;
28+
#[allow(unused_extern_crates)]
2729
extern crate rustc_plugin;
2830
#[allow(unused_extern_crates)]
2931
extern crate rustc_target;
@@ -144,6 +146,7 @@ pub mod methods;
144146
pub mod minmax;
145147
pub mod misc;
146148
pub mod misc_early;
149+
pub mod missing_const_for_fn;
147150
pub mod missing_doc;
148151
pub mod missing_inline;
149152
pub mod multiple_crate_versions;
@@ -478,6 +481,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
478481
reg.register_late_lint_pass(box redundant_clone::RedundantClone);
479482
reg.register_late_lint_pass(box slow_vector_initialization::Pass);
480483
reg.register_late_lint_pass(box types::RefToMut);
484+
reg.register_late_lint_pass(box missing_const_for_fn::MissingConstForFn);
481485

482486
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
483487
arithmetic::FLOAT_ARITHMETIC,
@@ -1017,6 +1021,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
10171021
reg.register_lint_group("clippy::nursery", Some("clippy_nursery"), vec![
10181022
attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
10191023
fallible_impl_from::FALLIBLE_IMPL_FROM,
1024+
missing_const_for_fn::MISSING_CONST_FOR_FN,
10201025
mutex_atomic::MUTEX_INTEGER,
10211026
needless_borrow::NEEDLESS_BORROW,
10221027
redundant_clone::REDUNDANT_CLONE,
+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use rustc::hir;
2+
use rustc::hir::{Body, FnDecl, Constness};
3+
use rustc::hir::intravisit::FnKind;
4+
// use rustc::mir::*;
5+
use syntax::ast::{NodeId, Attribute};
6+
use syntax_pos::Span;
7+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
8+
use rustc::{declare_tool_lint, lint_array};
9+
use rustc_mir::transform::qualify_min_const_fn::is_min_const_fn;
10+
use crate::utils::{span_lint, is_entrypoint_fn};
11+
12+
/// **What it does:**
13+
///
14+
/// Suggests the use of `const` in functions and methods where possible
15+
///
16+
/// **Why is this bad?**
17+
/// Not using `const` is a missed optimization. Instead of having the function execute at runtime,
18+
/// when using `const`, it's evaluated at compiletime.
19+
///
20+
/// **Known problems:**
21+
///
22+
/// Const functions are currently still being worked on, with some features only being available
23+
/// on nightly. This lint does not consider all edge cases currently and the suggestions may be
24+
/// incorrect if you are using this lint on stable.
25+
///
26+
/// Also, the lint only runs one pass over the code. Consider these two non-const functions:
27+
///
28+
/// ```rust
29+
/// fn a() -> i32 { 0 }
30+
/// fn b() -> i32 { a() }
31+
/// ```
32+
///
33+
/// When running Clippy, the lint will only suggest to make `a` const, because `b` at this time
34+
/// can't be const as it calls a non-const function. Making `a` const and running Clippy again,
35+
/// will suggest to make `b` const, too.
36+
///
37+
/// **Example:**
38+
///
39+
/// ```rust
40+
/// fn new() -> Self {
41+
/// Self {
42+
/// random_number: 42
43+
/// }
44+
/// }
45+
/// ```
46+
///
47+
/// Could be a const fn:
48+
///
49+
/// ```rust
50+
/// const fn new() -> Self {
51+
/// Self {
52+
/// random_number: 42
53+
/// }
54+
/// }
55+
/// ```
56+
declare_clippy_lint! {
57+
pub MISSING_CONST_FOR_FN,
58+
nursery,
59+
"Lint functions definitions that could be made `const fn`"
60+
}
61+
62+
#[derive(Clone)]
63+
pub struct MissingConstForFn;
64+
65+
impl LintPass for MissingConstForFn {
66+
fn get_lints(&self) -> LintArray {
67+
lint_array!(MISSING_CONST_FOR_FN)
68+
}
69+
}
70+
71+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn {
72+
fn check_fn(
73+
&mut self,
74+
cx: &LateContext<'_, '_>,
75+
kind: FnKind<'_>,
76+
_: &FnDecl,
77+
_: &Body,
78+
span: Span,
79+
node_id: NodeId
80+
) {
81+
let def_id = cx.tcx.hir().local_def_id(node_id);
82+
let mir = cx.tcx.optimized_mir(def_id);
83+
if let Err((span, err) = is_min_const_fn(cx.tcx, def_id, &mir) {
84+
cx.tcx.sess.span_err(span, &err);
85+
} else {
86+
match kind {
87+
FnKind::ItemFn(name, _generics, header, _vis, attrs) => {
88+
if !can_be_const_fn(&name.as_str(), header, attrs) {
89+
return;
90+
}
91+
},
92+
FnKind::Method(ident, sig, _vis, attrs) => {
93+
let header = sig.header;
94+
let name = ident.name.as_str();
95+
if !can_be_const_fn(&name, header, attrs) {
96+
return;
97+
}
98+
},
99+
_ => return
100+
}
101+
span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a const_fn");
102+
}
103+
}
104+
}
105+
106+
fn can_be_const_fn(name: &str, header: hir::FnHeader, attrs: &[Attribute]) -> bool {
107+
// Main and custom entrypoints can't be `const`
108+
if is_entrypoint_fn(name, attrs) { return false }
109+
110+
// We don't have to lint on something that's already `const`
111+
if header.constness == Constness::Const { return false }
112+
true
113+
}

clippy_lints/src/utils/mod.rs

+13
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,19 @@ pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option<Vec<&'a
330330
Some(matched)
331331
}
332332

333+
/// Returns true if the function is an entrypoint to a program
334+
///
335+
/// This is either the usual `main` function or a custom function with the `#[start]` attribute.
336+
pub fn is_entrypoint_fn(fn_name: &str, attrs: &[ast::Attribute]) -> bool {
337+
338+
let is_custom_entrypoint = attrs.iter().any(|attr| {
339+
attr.path.segments.len() == 1
340+
&& attr.path.segments[0].ident.to_string() == "start"
341+
});
342+
343+
is_custom_entrypoint || fn_name == "main"
344+
}
345+
333346
/// Get the name of the item the expression is in, if available.
334347
pub fn get_item_name(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<Name> {
335348
let parent_id = cx.tcx.hir().get_parent(expr.id);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//! False-positive tests to ensure we don't suggest `const` for things where it would cause a
2+
//! compilation error.
3+
//! The .stderr output of this test should be empty. Otherwise it's a bug somewhere.
4+
5+
#![warn(clippy::missing_const_for_fn)]
6+
#![feature(start)]
7+
8+
struct Game;
9+
10+
// This should not be linted because it's already const
11+
const fn already_const() -> i32 { 32 }
12+
13+
impl Game {
14+
// This should not be linted because it's already const
15+
pub const fn already_const() -> i32 { 32 }
16+
}
17+
18+
// Allowing on this function, because it would lint, which we don't want in this case.
19+
#[allow(clippy::missing_const_for_fn)]
20+
fn random() -> u32 { 42 }
21+
22+
// We should not suggest to make this function `const` because `random()` is non-const
23+
fn random_caller() -> u32 {
24+
random()
25+
}
26+
27+
static Y: u32 = 0;
28+
29+
// We should not suggest to make this function `const` because const functions are not allowed to
30+
// refer to a static variable
31+
fn get_y() -> u32 {
32+
Y
33+
//~^ ERROR E0013
34+
}
35+
36+
// Also main should not be suggested to be made const
37+
fn main() {
38+
// We should also be sure to not lint on closures
39+
let add_one_v2 = |x: u32| -> u32 { x + 1 };
40+
}
41+
42+
trait Foo {
43+
// This should not be suggested to be made const
44+
// (rustc restriction)
45+
fn f() -> u32;
46+
}
47+
48+
// Don't lint custom entrypoints either
49+
#[start]
50+
fn init(num: isize, something: *const *const u8) -> isize { 1 }

tests/ui/missing_const_for_fn/cant_be_const.stderr

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#![warn(clippy::missing_const_for_fn)]
2+
#![allow(clippy::let_and_return)]
3+
4+
use std::mem::transmute;
5+
6+
struct Game {
7+
guess: i32,
8+
}
9+
10+
impl Game {
11+
// Could be const
12+
pub fn new() -> Self {
13+
Self {
14+
guess: 42,
15+
}
16+
}
17+
}
18+
19+
// Could be const
20+
fn one() -> i32 { 1 }
21+
22+
// Could also be const
23+
fn two() -> i32 {
24+
let abc = 2;
25+
abc
26+
}
27+
28+
// TODO: Why can this be const? because it's a zero sized type?
29+
// There is the `const_string_new` feature, but it seems that this already works in const fns?
30+
fn string() -> String {
31+
String::new()
32+
}
33+
34+
// Could be const
35+
unsafe fn four() -> i32 { 4 }
36+
37+
// Could also be const
38+
fn generic<T>(t: T) -> T {
39+
t
40+
}
41+
42+
// FIXME: This could be const but is currently not linted
43+
fn sub(x: u32) -> usize {
44+
unsafe { transmute(&x) }
45+
}
46+
47+
// FIXME: This could be const but is currently not linted
48+
fn generic_arr<T: Copy>(t: [T; 1]) -> T {
49+
t[0]
50+
}
51+
52+
// Should not be const
53+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
error: this could be a const_fn
2+
--> $DIR/could_be_const.rs:12:5
3+
|
4+
LL | / pub fn new() -> Self {
5+
LL | | Self {
6+
LL | | guess: 42,
7+
LL | | }
8+
LL | | }
9+
| |_____^
10+
|
11+
= note: `-D clippy::missing-const-for-fn` implied by `-D warnings`
12+
13+
error: this could be a const_fn
14+
--> $DIR/could_be_const.rs:20:1
15+
|
16+
LL | fn one() -> i32 { 1 }
17+
| ^^^^^^^^^^^^^^^^^^^^^
18+
19+
error: this could be a const_fn
20+
--> $DIR/could_be_const.rs:30:1
21+
|
22+
LL | / fn string() -> String {
23+
LL | | String::new()
24+
LL | | }
25+
| |_^
26+
27+
error: this could be a const_fn
28+
--> $DIR/could_be_const.rs:35:1
29+
|
30+
LL | unsafe fn four() -> i32 { 4 }
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
33+
error: this could be a const_fn
34+
--> $DIR/could_be_const.rs:38:1
35+
|
36+
LL | / fn generic<T>(t: T) -> T {
37+
LL | | t
38+
LL | | }
39+
| |_^
40+
41+
error: aborting due to 5 previous errors
42+

0 commit comments

Comments
 (0)