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

Improve invalid_reference_casting lint #114784

Merged

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Aug 13, 2023

This PR improves the invalid_reference_casting lint:

  • by considering an unlimited number of casts instead only const to mut ptr
  • by also considering ptr-to-integer and integer-to-ptr casts
  • by also taking into account ptr::cast, ptr::cast and ptr::cast_const

Most of this improvements comes from skimming Github Code Search result for &mut \*.*as \*const

r? @est31 (maybe)

@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 13, 2023
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the many-improve-invalid_reference_casting-lint branch from 5cb4dde to 7c8dc1d Compare August 13, 2023 15:49
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the many-improve-invalid_reference_casting-lint branch 2 times, most recently from edebb8a to 6ed59b7 Compare August 14, 2023 09:50
@@ -112,6 +112,7 @@ impl<T: ?Sized> *mut T {
/// [`cast_mut`]: #method.cast_mut
#[stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_diagnostic_item = "ptr_cast_const"]
Copy link
Member

Choose a reason for hiding this comment

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

I think there is high confusion potential between ptr_const_cast and ptr_cast_const. Maybe this one could be renamed to ptr_mut_cast_const?

Copy link
Member Author

@Urgau Urgau Aug 15, 2023

Choose a reason for hiding this comment

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

I decided to rename the other one from ptr_const_cast to const_ptr_cast, so that every diagnostic item finishes with the name of it's method.

Copy link
Member

Choose a reason for hiding this comment

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

I think that rename improved the situation but the two names are still only distinguished by the order of the components. I think the mut should still be added to this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Except that the const part is now on the opposite side of the word: in front for const_ptr_cast and behind for ptr_cast_const, there is still the possibility that someone might mistake one for the other, but I think it's sufficiently diffused.

PS: I'm not against changing it, but I think adding mut would make the name out of place, and could definitely confuse a future me.

Copy link
Member

Choose a reason for hiding this comment

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

Mhh yeah the other names are also not prefixed with mut_, even though IMO they should be. I will approve this.

@Urgau Urgau force-pushed the many-improve-invalid_reference_casting-lint branch from 6ed59b7 to 91b05f8 Compare August 15, 2023 08:15
@est31
Copy link
Member

est31 commented Aug 16, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 16, 2023

📌 Commit 91b05f8 has been approved by est31

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 16, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 16, 2023
…ce_casting-lint, r=est31

Improve `invalid_reference_casting` lint

This PR improves the `invalid_reference_casting` lint:
 - by considering an unlimited number of casts instead only const to mut ptr
 - by also considering ptr-to-integer and integer-to-ptr casts
 - by also taking into account [`ptr::cast`](https://doc.rust-lang.org/std/primitive.pointer.html#method.cast), [`ptr::cast`](https://doc.rust-lang.org/std/primitive.pointer.html#method.cast-1) and [`ptr::cast_const`](https://doc.rust-lang.org/std/primitive.pointer.html#method.cast_const)

Most of this improvements comes from skimming Github Code Search result for [`&mut \*.*as \*const`](https://github.com/search?q=lang%3Arust+%2F%26mut+%5C*.*as+%5C*const%2F&type=code)

r? `@est31` (maybe)
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#113115 (we are migrating to askama)
 - rust-lang#114784 (Improve `invalid_reference_casting` lint)
 - rust-lang#114822 (Improve code readability by moving fmt args directly into the string)
 - rust-lang#114878 (rustc book: make more pleasant to search)
 - rust-lang#114899 (Add missing Clone/Debug impls to SMIR Trait related tys)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2667d85 into rust-lang:master Aug 16, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Aug 16, 2023
@Urgau Urgau deleted the many-improve-invalid_reference_casting-lint branch August 17, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants