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

Cleanup const_float_classify #13510

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alex-semenyuk
Copy link
Member

@alex-semenyuk alex-semenyuk commented Oct 6, 2024

As mentioned at #13508 const_float_classify has been stabilized recently in rust-lang/rust#130157 and can be cleanup

Close #13508

changelog: [none]

@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2024

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 6, 2024
@alex-semenyuk alex-semenyuk changed the title Clean up const_float_classify Cleanup const_float_classify Oct 6, 2024
&& (
matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst)
|| cx.tcx.features().declared(sym!(const_float_classify))
)
&& matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst)
Copy link
Member

Choose a reason for hiding this comment

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

If I understand the issue correctly, it sounds like we should rather replace the cx.tcx.features().declared(...) check with an MSRV check, since these functions are now stable in const.

With the current diff, we would never emit a warning in const code

Comment on lines -43 to 47
// Will be linted if `const_float_classify` is enabled
if const { X == f64::INFINITY || X == f64::NEG_INFINITY } {}
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have a test where this now emits a warning. Something like

const {
  let x = 1.0f64;
  if x == f64::INFINITY || x == f64::NEG_INFINITY {}
}

should suffice I think

@alex-semenyuk alex-semenyuk marked this pull request as draft October 9, 2024 21:01
@alex-semenyuk alex-semenyuk force-pushed the cleanup_const_float_classify branch 2 times, most recently from 55d1e88 to 3f68c41 Compare October 10, 2024 08:23
@alex-semenyuk alex-semenyuk marked this pull request as ready for review October 10, 2024 17:21
@llogiq
Copy link
Contributor

llogiq commented Oct 13, 2024

Looks good to me. Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 13, 2024

📌 Commit cb19f23 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 13, 2024

⌛ Testing commit cb19f23 with merge 04849bd...

bors added a commit that referenced this pull request Oct 13, 2024
…llogiq

Cleanup `const_float_classify`

As mentioned at #13508 `const_float_classify` has been stabilized recently in rust-lang/rust#130157 and can be cleanup

Close #13508

changelog: [none]
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.

Clean up const_float_classify leftovers
5 participants