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

Revert: or_fun_call should lint calls to const fns with no args #6077

Merged
merged 2 commits into from
Sep 24, 2020

Conversation

ebroto
Copy link
Member

@ebroto ebroto commented Sep 22, 2020

The changes in #5889 and #5984 were done under the incorrect assumption that a const fn with no args was guaranteed to be evaluated at compile time. A const fn is only guaranteed to be evaluated at compile time if it's inside a const context (the initializer of a const or a static).

See this zulip conversation for more details on this common misconception.

Given that none of the linted methods by or_fun_call can be called in const contexts, the lint should make no exceptions.

changelog: [or_fun_call] lints again calls to const fn with no args

@rust-highfive
Copy link

r? @yaahc

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 22, 2020
@matthiaskrgr
Copy link
Member

Just making sure I understood correctly;
The changes that are being reverted here were based on the assumption that const fns were always evaluated at compiletime BUT this is wrong because they are only evaluated at compile time if called from a const context (such as const var bindings)?

@ebroto
Copy link
Member Author

ebroto commented Sep 24, 2020

@matthiaskrgr that is my understanding, yep

@matthiaskrgr
Copy link
Member

Ok 👍

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 24, 2020

📌 Commit ce83d8d has been approved by matthiaskrgr

@bors
Copy link
Collaborator

bors commented Sep 24, 2020

⌛ Testing commit ce83d8d with merge cc1998f...

@matthiaskrgr
Copy link
Member

How urgent is this, should this be included in rust-lang/rust#77144 ? (pr was not approved yet)

@bors
Copy link
Collaborator

bors commented Sep 24, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: matthiaskrgr
Pushing cc1998f to master...

@bors bors merged commit cc1998f into rust-lang:master Sep 24, 2020
@ebroto ebroto deleted the revert_or_fun_call_const branch September 24, 2020 18:18
@ebroto
Copy link
Member Author

ebroto commented Sep 24, 2020

How urgent is this, should this be included in rust-lang/rust#77144

I think this can wait as it's just avoiding false negatives

lopopolo added a commit to artichoke/artichoke that referenced this pull request Dec 24, 2020
Disabling `clippy::or_fun_call` lint for calls to const functions was
done with an incorrect understanding that const fns with non arguments
would be guaranteed to be const evaluated. This is not the case. See
rust-lang/rust-clippy#6077.

The revert of this behavior in `clippy::or_fun_call` will land in Rust
1.49.0 due out in a week. This commit removes the pragmas to disable the
lint and modifies all callsites to use the lazy `ok_or_else` variant.

This commit reverts a change made in #794 and 9a8c14c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants