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

New lint for as *const _ and as *mut _ pointer casts #13251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Soveu
Copy link
Contributor

@Soveu Soveu commented Aug 10, 2024

changelog: New lint for as *const _ and as *mut _ pointer casts

EDIT: lets go with the simpler version

@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Jarcho (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 10, 2024
@Soveu
Copy link
Contributor Author

Soveu commented Aug 10, 2024

Or maybe we should just make a lint for as *const/*mut _, like for as _?

Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

The changes to the borrow_deref_ref tests should be reverted with the lint being allowed instead.

It's probably fine to lint on just &x as *const _, nut that would need to be paired with a check on the type required. You can use expr_use_ctxt to start on that.

Can you please change to using let-chains to match clippy's general style and tone the commenting down a bit. The current comments make the code harder to read.

@Soveu Soveu changed the title [WIP] New lint for accidental double pointer into flat addr cast New lint for as *const _ and as *mut _ pointer casts Aug 24, 2024
@Soveu
Copy link
Contributor Author

Soveu commented Aug 24, 2024

Should this lint be enabled by default like as_underscore? This would break some tests:

FAILURES:
    tests/ui/as_ptr_cast_mut.rs
    tests/ui/borrow_deref_ref.rs
    tests/ui/cast_slice_different_sizes.rs
    tests/ui/manual_c_str_literals.rs
    tests/ui/or_fun_call.rs
    tests/ui/ptr_eq.rs
    tests/ui/ptr_eq_no_std.rs
    tests/ui/ptr_as_ptr.rs
    tests/ui/ref_as_ptr.rs
    tests/ui/transmute_null_to_fn.rs
    tests/ui/transmuting_null.rs
    tests/ui/unnecessary_cast.rs
    tests/ui/zero_ptr_no_std.rs
    tests/ui/zero_ptr.rs
    tests/ui/author/issue_3849.rs

fn f(s: &S) -> usize {
&s as *const _ as usize
//~^ ERROR: using inferred pointer cast
//FIXME: idk how to get this help working ~| HELP: use explicit type: `*const &S`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How it should be done?

Copy link
Contributor

@Jarcho Jarcho Sep 11, 2024

Choose a reason for hiding this comment

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

Sorry for the long delay. I've had a busy few weeks.

Since this is a pointer to a reference you can suggest as *const &_ rather than filling in the whole type. If you ever need it Ty<'_> implements Display.

Looks like this changed to just linting as *_. You can just get the pointee type and print it since Ty<'_> implements Display.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, the help message is printed, but I can't get it to work with ~| HELP: ... matching here specifically. Sorry for not being precise

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, just looked at the new code now. Are you referring to the formatting of the message? You'll have to use span_lint_and_then and the span_suggestion_with_style. I think we'll be changing the default suggestion style at some point in the future.

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.

3 participants