Skip to content

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

@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

@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
Contributor

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
Contributor

bors commented Oct 18, 2023

📌 Commit 80a092c has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 18, 2023

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

@bors
Copy link
Contributor

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
@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