Add initial version of const_fn lint #3648

phansch · 2019-01-09T19:14:19Z

This adds an initial version of a lint that can tell if a function could be const.

TODO:

Finish up the docs
Fix the ICE

cc #2440

phansch · 2019-01-09T19:24:02Z

Soo... some questions:

Is this a good name for the lint?
There are two FIXMEs in the tests where is_min_const_fn doesn't detect that the function can be const. I would leave them in to be fixed later?
Did I miss any obvious test cases? I suppose some more complex test cases would be nice.

phansch · 2019-01-09T20:39:51Z

Lots of CI failures, so I'm going to have to do some more work first before it's ready for review.

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

clippy_lints/src/missing_const_for_fn.rs

oli-obk · 2019-01-11T11:10:44Z

clippy_lints/src/missing_const_for_fn.rs

+                        return;
+                    }
+                },
+                FnKind::Method(ident, sig, _vis, attrs) => {


I think this will also trigger on methods in traits with default bodies

Does that matter though? Trait methods with default bodies can't be const anyway , according to the RFC.

Right, what I meant was that the code looks to me like it will suggest to make trait methods with default bodies const fn

tests/ui/missing_const_for_fn/cant_be_const.rs

This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per rust-lang/rust-clippy#3648 (comment)

@oli-obk

Remove delay_span_bug from qualify_min_const_fn This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per rust-lang/rust-clippy#3648 (comment) r? @oli-obk

clippy_lints/src/utils/mod.rs

tests/ui/missing_const_for_fn/could_be_const.rs

phansch · 2019-01-23T07:01:54Z

I think this is ready for another review now

oli-obk · 2019-01-23T11:33:36Z

clippy_lints/src/missing_const_for_fn.rs

+///
+/// **Why is this bad?**
+///
+/// Not using `const` is a missed optimization. Instead of having the function execute at runtime,


That's not quite true. This optimization will still happen for non-const fn. The major use case is crates forgetting to use const fn will unnecessarily limit their consumers in what they can do. This is similar to forgetting to implement PartialEq or similar things on types where it might be helpful to have that.

oli-obk · 2019-01-23T11:38:18Z

r=me with documentation nit addressed

bors · 2019-01-23T18:04:35Z

☔ The latest upstream changes (presumably #3582) made this pull request unmergeable. Please resolve the merge conflicts.

* `const_transmute` currently also seems to depend on the `const_fn` feature. * Only `Sized` is currently allowed as a bound, not Copy.

phansch · 2019-01-29T19:27:55Z

r=me with documentation nit addressed

oh I missed that comment 👀

@bors r=oli-obk

bors · 2019-01-29T19:27:56Z

📌 Commit d0d7c5e has been approved by oli-obk

bors · 2019-01-29T19:58:19Z

⌛ Testing commit d0d7c5e with merge 6b1a2a9...

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 - [x] Fix the ICE cc #2440

bors · 2019-01-29T20:41:32Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 6b1a2a9 to master...

This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per rust-lang/rust-clippy#3648 (comment)

phansch changed the title ~~Add initial version of const_fn lint~~ WIP: Add initial version of const_fn lint Jan 9, 2019

phansch force-pushed the const_fn_lint branch from f17ef6d to 69d423b Compare January 9, 2019 19:28

phansch changed the title ~~WIP: Add initial version of const_fn lint~~ Add initial version of const_fn lint Jan 9, 2019

phansch force-pushed the const_fn_lint branch from 69d423b to 2ce631e Compare January 9, 2019 20:47

This comment has been minimized.

Sign in to view

oli-obk reviewed Jan 11, 2019

View reviewed changes

clippy_lints/src/missing_const_for_fn.rs Outdated Show resolved Hide resolved

oli-obk reviewed Jan 11, 2019

View reviewed changes

tests/ui/missing_const_for_fn/cant_be_const.rs Show resolved Hide resolved

phansch added a commit to phansch/rust that referenced this pull request Jan 18, 2019

Remove delay_span_bug from qualify_min_const_fn

79ef971

This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per rust-lang/rust-clippy#3648 (comment)

phansch mentioned this pull request Jan 18, 2019

Remove delay_span_bug from qualify_min_const_fn rust-lang/rust#57736

Merged

phansch force-pushed the const_fn_lint branch 3 times, most recently from fdc3f46 to cadac5b Compare January 21, 2019 07:41

oli-obk reviewed Jan 21, 2019

View reviewed changes

phansch added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jan 23, 2019

oli-obk reviewed Jan 23, 2019

View reviewed changes

phansch force-pushed the const_fn_lint branch from 9de367d to 101d96d Compare January 23, 2019 20:15

phansch force-pushed the const_fn_lint branch 4 times, most recently from 97653e0 to 0bec822 Compare January 29, 2019 06:00

phansch added 6 commits January 29, 2019 08:19

Add initial version of const_fn lint

c3980bf

Maybe fix ICE?

68cc4df

Reorganize conditionals: Run faster checks first

f9d65b6

cargo fmt

c0a0269

Use built-in entry_fn detection over self-built

0c6bdda

Update various docs

aed001b

* `const_transmute` currently also seems to depend on the `const_fn` feature. * Only `Sized` is currently allowed as a bound, not Copy.

phansch force-pushed the const_fn_lint branch from 0bec822 to 92bb817 Compare January 29, 2019 07:19

cargo fmt

d0d7c5e

phansch force-pushed the const_fn_lint branch from 92bb817 to d0d7c5e Compare January 29, 2019 18:56

bors merged commit d0d7c5e into rust-lang:master Jan 29, 2019

phansch deleted the const_fn_lint branch January 29, 2019 20:42

VardhanThigle pushed a commit to jethrogb/rust that referenced this pull request Jan 31, 2019

Remove delay_span_bug from qualify_min_const_fn

1797083

This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per rust-lang/rust-clippy#3648 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial version of const_fn lint #3648

Add initial version of const_fn lint #3648

phansch commented Jan 9, 2019 •

edited

Loading

phansch commented Jan 9, 2019

phansch commented Jan 9, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk Jan 11, 2019

phansch Jan 14, 2019

oli-obk Jan 14, 2019

phansch commented Jan 23, 2019

oli-obk Jan 23, 2019

oli-obk commented Jan 23, 2019

bors commented Jan 23, 2019

phansch commented Jan 29, 2019

bors commented Jan 29, 2019

bors commented Jan 29, 2019

bors commented Jan 29, 2019

Add initial version of const_fn lint #3648

Add initial version of const_fn lint #3648

Conversation

phansch commented Jan 9, 2019 • edited Loading

phansch commented Jan 9, 2019

phansch commented Jan 9, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk Jan 11, 2019

Choose a reason for hiding this comment

phansch Jan 14, 2019

Choose a reason for hiding this comment

oli-obk Jan 14, 2019

Choose a reason for hiding this comment

phansch commented Jan 23, 2019

oli-obk Jan 23, 2019

Choose a reason for hiding this comment

oli-obk commented Jan 23, 2019

bors commented Jan 23, 2019

phansch commented Jan 29, 2019

bors commented Jan 29, 2019

bors commented Jan 29, 2019

bors commented Jan 29, 2019

phansch commented Jan 9, 2019 •

edited

Loading