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

needless_borrows_for_generic_args: Handle when field operand impl Drop #11900

Merged
merged 1 commit into from Dec 5, 2023

Conversation

Enselic
Copy link
Member

@Enselic Enselic commented Dec 1, 2023

Before this fix, the lint had a false positive, namely when a reference was taken to a field when the field operand implements a custom Drop. The compiler will refuse to partially move a type that implements Drop, because that would put the type in a weird state.

False Positive Example (Fixed)

struct CustomDrop(String);

impl Drop for CustomDrop {
    fn drop(&mut self) {}
}

fn check_str<P: AsRef<str>>(_to: P) {}

fn test() {
    let owner = CustomDrop(String::default());
    check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
}

changelog: [needless_borrows_for_generic_args]: Handle when field operand impl Drop

Before this fix, the lint had a false positive, namely when a reference
was taken to a field when the field operand implements a custom Drop.
The compiler will refuse to partially move a type that implements Drop,
because that would put the operand in a weird state. See added
regression test.
@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

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 Dec 1, 2023
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2023

📌 Commit 512f302 has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 2, 2023

⌛ Testing commit 512f302 with merge 6289c30...

bors added a commit that referenced this pull request Dec 2, 2023
needless_borrows_for_generic_args: Handle when field operand impl Drop

Before this fix, the lint had a false positive, namely when a reference was taken to a field when the field operand implements a custom Drop. The compiler will refuse to partially move a type that implements Drop, because that would put the type in a weird state.

## False Positive Example (Fixed)

```rs
struct CustomDrop(String);

impl Drop for CustomDrop {
    fn drop(&mut self) {}
}

fn check_str<P: AsRef<str>>(_to: P) {}

fn test() {
    let owner = CustomDrop(String::default());
    check_str(&owner.0); // Don't lint. `owner` can't be partially moved because it impl Drop
}
```

changelog: [`needless_borrows_for_generic_args`]: Handle when field operand impl Drop
@bors
Copy link
Collaborator

bors commented Dec 2, 2023

💔 Test failed - checks-action_test

@pgerber
Copy link
Contributor

pgerber commented Dec 2, 2023

Looks like the same error I'm seeing in my pull request. See #11904 (comment).

@matthiaskrgr
Copy link
Member

@bors r=Manishearth

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

📌 Commit 512f302 has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

⌛ Testing commit 512f302 with merge 4a56563...

@bors
Copy link
Collaborator

bors commented Dec 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 4a56563 to master...

@bors bors merged commit 4a56563 into rust-lang:master Dec 5, 2023
5 checks passed
Enselic added a commit to Enselic/cargo-public-api that referenced this pull request Mar 5, 2024
My [fix](rust-lang/rust-clippy#11900) for the
false positive has reached Rust 1.76 stable, so let's remove the
annoying `allow`.
Enselic added a commit to Enselic/cargo-public-api that referenced this pull request Mar 5, 2024
My [fix](rust-lang/rust-clippy#11900) for the
false positive has reached Rust 1.76 stable, so let's remove the
annoying `allow`.
@Enselic Enselic deleted the needless-borrow-drop branch March 9, 2024 15:46
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.

None yet

6 participants