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

Special-case suggestions for null pointers constness cast #13369

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

samueltardieu
Copy link
Contributor

This implements the suggestions from #13361. It fits into the existing ptr_cast_constness lint, as this is a specialized version. However,

  1. I have not modified the lint MSRV, so the documentation for this lint will still show that it applies only from Rust 1.72.0. This is true in the general case, but the lint for null pointers will trigger even before this version as null() and null_mut() were already present in Rust 1.0 and there is no reason not to apply this lint. I guess this is only a minor documentation issue that can be ignored.
  2. I have not covered the core::ptr::null::<T>().cast_mut() (could be made into core::ptr::null_mut::<T>()) and cotr::ptr::null_mut::<T>().cast_const() (could be made into core::ptr::null::<T>()) cases. Should they be covered? If they should, here or in a separate PR?

changelog: [ptr_cast_constness]: special-case suggestions for null pointers constness cast

Fix #13361

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2024

r? @y21

rustbot has assigned @y21.
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 Sep 7, 2024
@y21
Copy link
Member

y21 commented Sep 7, 2024

For the second point, it sounds reasonable to me to lint on that too as part of the same lint (although the description would then need to be adapted a fair bit since it's drifting away from the original intent of linting on as casts specifically). Doesn't need to be done in this PR and could be a followup if you want to do that also, up to you. The linked issue also doesn't mention it and this is already an improvement on its own.

@samueltardieu
Copy link
Contributor Author

For the second point, it sounds reasonable to me to lint on that too as part of the same lint (although the description would then need to be adapted a fair bit since it's drifting away from the original intent of linting on as casts specifically). Doesn't need to be done in this PR and could be a followup if you want to do that also, up to you. The linked issue also doesn't mention it and this is already an improvement on its own.

I've added it in a second commit.

clippy_lints/src/casts/mod.rs Show resolved Hide resolved
clippy_lints/src/casts/ptr_cast_constness.rs Outdated Show resolved Hide resolved
clippy_lints/src/casts/ptr_cast_constness.rs Outdated Show resolved Hide resolved
@samueltardieu samueltardieu force-pushed the issue-13361 branch 4 times, most recently from bb52c83 to 99e497c Compare September 9, 2024 07:21
This covers two cases:

- `core::ptr::null::<T>().cast_mut()` -> `core::ptr::null_mut::<T>()`
- `core::ptr::null_mut::<T>().cast_const()` -> `core::ptr::null::<T>()`
@samueltardieu
Copy link
Contributor Author

samueltardieu commented Sep 9, 2024

Also, @y21, messages for std::ptr::null() as *mut and similar are more specific than the other messages in the same lint. Do you want me to unify them and keep only the "generic" one? ("changing constness of a null pointer")

Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

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

This looks good to me as is. I think having these specific lint messages is fine 👍

@y21
Copy link
Member

y21 commented Sep 11, 2024

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Sep 11, 2024

📌 Commit 3060873 has been approved by y21

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Sep 11, 2024

⌛ Testing commit 3060873 with merge 131681b...

@bors
Copy link
Contributor

bors commented Sep 11, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: y21
Pushing 131681b to master...

@bors bors merged commit 131681b into rust-lang:master Sep 11, 2024
8 checks passed
@samueltardieu samueltardieu deleted the issue-13361 branch September 22, 2024 15:10
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.

Suggest ptr::null_mut::<T>() instead of ptr::null::<T>() as *mut T
4 participants