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

Don't emit needless_pass_by_ref_mut if the variable is used in an unsafe block or function #11624

Merged

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Oct 6, 2023

Fixes #11586.
Fixes #11180.

As suggested in the two issues above, this lint should not be emitted if this an unsafe function or if the argument is used in an unsafe block.

changelog: [needless_pass_by_ref_mut]: Don't emit if the variable is used in an unsafe block or function

@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2023

r? @Jarcho

(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 Oct 6, 2023
@GuillaumeGomez
Copy link
Member Author

r? @Centri3

@rustbot rustbot assigned Centri3 and unassigned Jarcho Oct 6, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the needless_pass_by_ref_mut-unsafe-fn-block branch from 15d012c to a9c63ed Compare October 6, 2023 10:11
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.

Ignoring all unsafe functions is the right move. LGTM, just one nit

clippy_lints/src/needless_pass_by_ref_mut.rs Outdated Show resolved Hide resolved
@GuillaumeGomez GuillaumeGomez force-pushed the needless_pass_by_ref_mut-unsafe-fn-block branch from a9c63ed to 33c6e06 Compare October 6, 2023 12:12
@GuillaumeGomez
Copy link
Member Author

Updated!

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.

On a closer look these tests don't seem correct (& instead of &mut). Also I have a suggestion for the tests

tests/ui/needless_pass_by_ref_mut.rs Outdated Show resolved Hide resolved
@GuillaumeGomez GuillaumeGomez force-pushed the needless_pass_by_ref_mut-unsafe-fn-block branch from 33c6e06 to 6e6e267 Compare October 6, 2023 19:43
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Oct 6, 2023

Updated the test and fixed the code (the unsafe check was done on the wrong HirId).

@GuillaumeGomez
Copy link
Member Author

Seems like Centri3 is busy so I was recommended another reviewer.

r? @blyxyas

@rustbot rustbot assigned blyxyas and unassigned Centri3 Oct 17, 2023
@bors
Copy link
Collaborator

bors commented Oct 17, 2023

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

@GuillaumeGomez GuillaumeGomez force-pushed the needless_pass_by_ref_mut-unsafe-fn-block branch from 6e6e267 to 80a092c Compare October 17, 2023 13:37
@GuillaumeGomez
Copy link
Member Author

Fixed conflict.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ❤️

@blyxyas
Copy link
Member

blyxyas commented Oct 18, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 18, 2023

📌 Commit 80a092c has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 18, 2023

⌛ Testing commit 80a092c with merge 5fb312e...

@bors
Copy link
Collaborator

bors commented Oct 18, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 5fb312e to master...

@bors bors merged commit 5fb312e into rust-lang:master Oct 18, 2023
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the needless_pass_by_ref_mut-unsafe-fn-block branch October 18, 2023 18:00
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
6 participants