Avoid or_fun_call for const_fn with no args #5889
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @Manishearth (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ thanks! |
|
📌 Commit 5d66bd7 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
lopopolo
added a commit
to artichoke/artichoke
that referenced
this pull request
Aug 18, 2020
This commit makes as many methods const as possible. Most involve the modification or introduction of zero-arg `new()` methods. This commit makes all error ZSTs only constructable via `T::new` instead of directly instantiating the struct inline by adding a `_private: ()` field. This change inverts the implementation of `T::default` and `T::new` so that `T::default` is implemented with `T::new`, allowing `T::new` to be const. All callsites have been updated to prefer calling `T::new`. It does not appear possible to construct byte slice literals in const fn. Suppress `clippy::or_fun_call` lint errors until rust-lang/rust-clippy#5889 is included in stable Rust. This commit reworks the implementation of `regexp::Options` to store a custom `RegexpOption` enum with `Enabled` and `Disabled` states instead of a `bool`. This eliminates a suppressed clippy lint and makes the code more readable. There are `From` implementations to and from `bool` as well as `RegexpOption::is_enabled`. Much of these functions can become const once const if match is stable. This commit also includes some readability improvements to extracting `RClass` pointers via `with_ffi_boundary`.
lopopolo
added a commit
to artichoke/artichoke
that referenced
this pull request
Aug 18, 2020
This commit makes as many methods const as possible. Most involve the modification or introduction of zero-arg `new()` methods. This commit makes all error ZSTs only constructable via `T::new` instead of directly instantiating the struct inline by adding a `_private: ()` field. This change inverts the implementation of `T::default` and `T::new` so that `T::default` is implemented with `T::new`, allowing `T::new` to be const. All callsites have been updated to prefer calling `T::new`. It does not appear possible to construct byte slice literals in const fn. Suppress `clippy::or_fun_call` lint errors until rust-lang/rust-clippy#5889 is included in stable Rust. This commit reworks the implementation of `regexp::Options` to store a custom `RegexpOption` enum with `Enabled` and `Disabled` states instead of a `bool`. This eliminates a suppressed clippy lint and makes the code more readable. There are `From` implementations to and from `bool` as well as `RegexpOption::is_enabled`. Much of these functions can become const once const if match is stable. This commit also includes some readability improvements to extracting `RClass` pointers via `with_ffi_boundary`.
bors
added a commit
that referenced
this pull request
Sep 24, 2020
Revert: or_fun_call should lint calls to `const fn`s with no args 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](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Common.20misconception.3A.20.60const.20fn.60.20and.20its.20effect.20on.20codegen/near/208059113) 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #5682 by @lzutao
This avoids a subset of false positives, specifically those related to
const fns that take no arguments.For the rest, a much more involved fix would be needed, see #5682 (comment).
So this does not solve #5658
changelog: Avoid triggering [
or_fun_call] withconst fns that take no arguments.Fixes #5886