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.

Sign up for GitHub

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

Jump to bottom

Avoid invalid NaN lint machine-applicable suggestion in const context #114486

Merged
merged 2 commits into from
Aug 6, 2023
Merged

Avoid invalid NaN lint machine-applicable suggestion in const context #114486

merged 2 commits into from
Aug 6, 2023

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Aug 4, 2023 •
edited
Loading

This PR removes the machine-applicable suggestion in const context for the invalid_nan_comparision lint and replace it with a simple help.

Fixes #114471

@rustbot
Copy link
Collaborator

rustbot commented Aug 4, 2023

r? @davidtwco

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 4, 2023
@compiler-errors
Copy link
Member

I wonder if we should just see what's blocking stabilizing is_nan in const contexts 😆

@Urgau
Copy link
Member Author

Urgau commented Aug 5, 2023

I wonder if we should just see what's blocking stabilizing is_nan in const contexts laughing

The tracking issue #72505, say that it's blocked on #57241, ie general availability of floating point types in const fn.

@compiler-errors
Copy link
Member

Well in that case, shouldn't we just be suggesting nothing? I don't think turning this into a help that still tells folks to use the same fn is the right solution.

@Urgau Urgau force-pushed the const-context-nan-suggestion-114471 branch from cc264a1 to b71f2be Compare August 5, 2023 21:55
@Urgau
Copy link
Member Author

Urgau commented Aug 5, 2023 •
edited
Loading

It's a bit unsatisfying to not have any suggestion for const context, but I agree, better suggest nothing than suggest functions that have no plan to stabilization in the short or medium term.

Updated the PR to completely remove the suggestion.

compiler-errors
compiler-errors approved these changes Aug 6, 2023
View reviewed changes
compiler/rustc_lint/src/types.rs
e: &hir::Expr<'_>,
l: &hir::Expr<'_>,
r: &hir::Expr<'_>,
f: impl FnOnce(Span, Span) -> InvalidNanComparisonsSuggestion,
) -> InvalidNanComparisons {
let suggestion =
let suggestion = (!cx.tcx.hir().is_inside_const_context(e.hir_id)).then(|| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is a somewhat confusing way of avoiding an if statement 😅

compiler/rustc_lint/src/types.rs Show resolved Hide resolved
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 6, 2023

📌 Commit 3b3e466 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 6, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2023
@bors
Auto merge of rust-lang#114526 - matthiaskrgr:rollup-zz041wi, r=matth…
11467b1 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#114486 (Avoid invalid NaN lint machine-applicable suggestion in const context)
 - rust-lang#114503 (Remove invalid lint when there is a generic argument in prefix path)
 - rust-lang#114509 (Migrate GUI colors test to original CSS color format)
 - rust-lang#114524 (Also ICE when goals go from Ok to Err in new solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1305a43 into rust-lang:master Aug 6, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Aug 6, 2023
@Urgau Urgau deleted the const-context-nan-suggestion-114471 branch August 6, 2023 11:29
@RalfJung RalfJung mentioned this pull request Aug 3, 2024
Closed
@eduardosm eduardosm mentioned this pull request Sep 9, 2024
Merged
eduardosm added a commit to eduardosm/rust that referenced this pull request Sep 25, 2024
@eduardosm
Revert "Avoid invalid NaN lint machine-applicable suggestion in const…
c39ae56 
… context"

Reverts PR rust-lang#114486 (commit 1305a43)
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 4, 2024
@bors
Auto merge of rust-lang#130157 - eduardosm:stabilize-const_float_clas…
14f303b 
…sify, r=RalfJung

Stabilize `const_float_classify`

Tracking issue: rust-lang#72505

Also reverts rust-lang#114486

Closes rust-lang#72505

Stabilized const API:

```rust
impl f32 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}

impl f64 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}
```

cc `@rust-lang/wg-const-eval` `@rust-lang/libs-api`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Oct 8, 2024
@bors
Auto merge of #130157 - eduardosm:stabilize-const_float_classify, r=R…
d03c751 
…alfJung

Stabilize `const_float_classify`

Tracking issue: rust-lang/rust#72505

Also reverts rust-lang/rust#114486

Closes rust-lang/rust#72505

Stabilized const API:

```rust
impl f32 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}

impl f64 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}
```

cc `@rust-lang/wg-const-eval` `@rust-lang/libs-api`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Oct 14, 2024
@bors
Auto merge of #130157 - eduardosm:stabilize-const_float_classify, r=R…
a596ab1 
…alfJung

Stabilize `const_float_classify`

Tracking issue: rust-lang/rust#72505

Also reverts rust-lang/rust#114486

Closes rust-lang/rust#72505

Stabilized const API:

```rust
impl f32 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}

impl f64 {
    pub const fn is_nan(self) -> bool;
    pub const fn is_infinite(self) -> bool;
    pub const fn is_finite(self) -> bool;
    pub const fn is_subnormal(self) -> bool;
    pub const fn is_normal(self) -> bool;
    pub const fn classify(self) -> FpCategory;
    pub const fn is_sign_positive(self) -> bool;
    pub const fn is_sign_negative(self) -> bool;
}
```

cc `@rust-lang/wg-const-eval` `@rust-lang/libs-api`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@davidtwco davidtwco

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.

invalid_nan_comparisons suggests invalid 5f32.is_nan() in const context
5 participants
@Urgau @rustbot @compiler-errors @bors @davidtwco