Skip to content

Lint zero_ptr in const contexts #10212

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

Closed
wants to merge 3 commits into from
Closed

Conversation

Niki4tap
Copy link
Contributor

@Niki4tap Niki4tap commented Jan 18, 2023

Lint zero_ptr in const contexts, since core::ptr::null is now const stabilized.

This PR doesn't affect cmp_nan lint (aside from fixme comment) that was disabled as well, because {f32,f64}::is_nan is still unstable in const contexts, so this is essentially blocked on rust-lang/rust#57241


changelog: Enhancement: [zero_ptr]: Now lints in const contexts
#10212

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2023

r? @llogiq

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 18, 2023
@Niki4tap
Copy link
Contributor Author

Giving it another look, the fixme comment about cmp_nan doesn't really make sense, since the lint doesn't give you an automatically-applicable suggestion, and the note about {f32,f64}::is_nan isn't even correct in cases of NAN > some_float and other comparison operations, maybe the lint message should be reworked. I would be glad to do it if this is desirable.

@llogiq
Copy link
Contributor

llogiq commented Jan 21, 2023

Why did you remove the crashes test?

@Niki4tap
Copy link
Contributor Author

I think the test is mostly commented out and wasn't quite working anyways, and what was left of it was testing the behavior of zero_ptr and cmp_nan lints in const contexts (not sure why these checks are in that test in the first place), which this PR changes.

cmp_nan checks were moved to tests/ui/cmp_nan.rs.

@llogiq
Copy link
Contributor

llogiq commented Jan 21, 2023

Ok, looks good otherwise. We'll have to revisit once the Rust stabilization PR is merged.

@rustbot blocked

@rustbot rustbot added S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jan 21, 2023
@bors
Copy link
Contributor

bors commented May 8, 2023

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

@xFrednet
Copy link
Member

It looks like the feature is still blocking this PR and there is nothing indicating that it will be stabilized soon. I'll close this PR for now. Once the feature is stabilized, we can open a new PR to add this behavior.

@Niki4tap thank you for the time you put into this!

@xFrednet xFrednet closed this Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants