Skip to content

read_zero_byte_vec refactor for better heuristics #11766

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

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

dswij
Copy link
Member

@dswij dswij commented Nov 7, 2023

Fixes #9274

Previously, the implementation of read_zero_byte_vec only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor.

This PR refactors so that:

  1. It checks if there is a resize on the vec
  2. It works on blocks properly

e.g. This should properly lint now:

    let mut v = Vec::new();
    {
        f.read(&mut v)?;
        //~^ ERROR: reading zero byte data to `Vec`
    }

changelog: [read_zero_byte_vec] Refactored for better heuristics

@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2023

r? @Manishearth

(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 Nov 7, 2023
@dswij
Copy link
Member Author

dswij commented Nov 7, 2023

Let's hope that this is good enough to bring this lint back from nursery

Comment on lines -58 to -64
// should not lint
let mut empty = vec![];
let mut data7 = vec![];
f.read(&mut empty);

// should not lint
f.read(&mut data7);
Copy link
Member Author

Choose a reason for hiding this comment

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

This should lint, but the previous implementation can't catch this

@Manishearth
Copy link
Member

r? @Alexendoo

not going to have time this week I think

@rustbot rustbot assigned Alexendoo and unassigned Manishearth Nov 9, 2023
@dswij
Copy link
Member Author

dswij commented Dec 26, 2023

Friendly ping :) @Alexendoo

@dswij
Copy link
Member Author

dswij commented Jan 13, 2024

r? @blyxyas

since it seems like you have the least amount of pending reviews in queue (if you don't mind, of course)

@rustbot rustbot assigned blyxyas and unassigned Alexendoo Jan 13, 2024
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 Jan 17, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 17, 2024

📌 Commit 8c79f78 has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit 8c79f78 with merge 55fbd7d...

bors added a commit that referenced this pull request Jan 17, 2024
`read_zero_byte_vec` refactor for better heuristics

Fixes #9274

Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor.

This PR refactors so that:

1. It checks if there is a `resize`	on the vec
2. It works on blocks properly

e.g. This should properly lint now:

```
    let mut v = Vec::new();
    {
        f.read(&mut v)?;
        //~^ ERROR: reading zero byte data to `Vec`
    }
```

changelog: [`read_zero_byte_vec`] Refactored for better heuristics
@bors
Copy link
Contributor

bors commented Jan 17, 2024

💔 Test failed - checks-action_test

@blyxyas
Copy link
Member

blyxyas commented Jan 17, 2024

?
@bors retry

bors added a commit that referenced this pull request Jan 17, 2024
`read_zero_byte_vec` refactor for better heuristics

Fixes #9274

Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor.

This PR refactors so that:

1. It checks if there is a `resize`	on the vec
2. It works on blocks properly

e.g. This should properly lint now:

```
    let mut v = Vec::new();
    {
        f.read(&mut v)?;
        //~^ ERROR: reading zero byte data to `Vec`
    }
```

changelog: [`read_zero_byte_vec`] Refactored for better heuristics
@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit 8c79f78 with merge e4a07a4...

@bors
Copy link
Contributor

bors commented Jan 17, 2024

💔 Test failed - checks-action_test

@blyxyas
Copy link
Member

blyxyas commented Jan 17, 2024

I'll retry this one more time (if it doesn't work, we'll have to investigate further or wait a few hours), it seems like macos is having some sporadic failures.

@blyxyas
Copy link
Member

blyxyas commented Jan 17, 2024

@bors retry

@bors
Copy link
Contributor

bors commented Jan 17, 2024

⌛ Testing commit 8c79f78 with merge e27ebf2...

@bors
Copy link
Contributor

bors commented Jan 17, 2024

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

@bors bors merged commit e27ebf2 into rust-lang:master Jan 17, 2024
@dswij dswij deleted the issue-9274 branch January 22, 2024 10: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.

read_zero_byte_vec emitted when read is in the next block
6 participants