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

Look at adjusted types instead of fn signature types in ptr_arg #13313

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

y21
Copy link
Member

@y21 y21 commented Aug 27, 2024

This simplifies the implementation of the ptr_arg lint a bit and:
Fixes #13308
Fixes #10612

Currently, the lint checks if e.g. a &String parameter is only used in contexts where a &str could work. Part of it worked by looking for path exprs to that parameter in function call position specifically, looking at the callee signature and checking if that parameter type without refs is String (or one of a bunch of other special cases).

This simplified version removes the special casing of function calls and looking at the parameter type and instead just looks at the adjusted type, regardless of the expression it's in. This naturally also covers what the previous version was doing (if the expression is in a function call argument position expecting a &str, then it will have that adjustment. If it requires a &String then it won't have that adjustment and we won't lint).

The linked issue was a FP that happened because the previous implementation simply didn't consider projection types in the signature type, but with this simplification we don't really need to consider that (because we aren't looking at function signatures at all anymore -- the &mut Self::Owned parameter type of clone_into is the already-instantiated and normalized type &mut String).

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Aug 27, 2024

r? @Centri3

rustbot has assigned @Centri3.
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 Aug 27, 2024
@bors
Copy link
Collaborator

bors commented Sep 5, 2024

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

Copy link
Member

@Centri3 Centri3 left a comment

Choose a reason for hiding this comment

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

Looks perfect, just one thing!

clippy_lints/src/ptr.rs Outdated Show resolved Hide resolved
@Centri3
Copy link
Member

Centri3 commented Sep 13, 2024

Thanks! @bors r+

@bors
Copy link
Collaborator

bors commented Sep 13, 2024

📌 Commit 15495eb has been approved by Centri3

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 13, 2024

⌛ Testing commit 15495eb with merge 2536745...

@bors
Copy link
Collaborator

bors commented Sep 13, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Centri3
Pushing 2536745 to master...

@bors bors merged commit 2536745 into rust-lang:master Sep 13, 2024
8 checks passed
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.

ptr_arg triggers on &mut String despite clone_into ptr_arg false positive on ToOwned::clone_into()
4 participants