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

Avoid slice indexing in Clippy (down with the ICEs) #7638

Merged
merged 1 commit into from
Sep 5, 2021

Conversation

xFrednet
Copy link
Member

@xFrednet xFrednet commented Sep 5, 2021

While working on #7569 I got about 23 lint reports where we can avoid slice indexing by destructing the slice early. This is a preparation PR to avoid fixing them in the lint PR. (The implementation already takes about 300 lines without tests 😅). Either way, this should hopefully be easy to review 🙃


changelog: none

@rust-highfive
Copy link

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 5, 2021
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 5, 2021

📌 Commit 62b4612 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Sep 5, 2021

⌛ Testing commit 62b4612 with merge 051286d...

@bors
Copy link
Contributor

bors commented Sep 5, 2021

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

@bors bors merged commit 051286d into rust-lang:master Sep 5, 2021
@xFrednet xFrednet deleted the 7569-avoid-indexing-in-clippy branch September 5, 2021 17:43
bors added a commit that referenced this pull request Nov 11, 2021
New lint `avoidable_slice_indexing` to avoid slice indexing

A new lint to check for slices that could be deconstructed to avoid indexing. This lint should hopefully prevent some panics in other projects and ICEs for us. See #7569 for an example

The implementation specifically checks for immutable bindings in `if let` statements to slices and arrays. Then it checks if these bindings are only used for value access using indices and that these indices are lower than the configured limit. I did my best to keep the implementation small, however the check was sadly quite complex. Now it's around 300 lines for the implementation and the rest are test.

---

Optional future improvements:
* Check for these instances also in `match` statements
* Check for mutable slice bindings that could also be destructed

---

changelog: New lint [`avoidable_slice_indexing`]

I've already fixed a bunch of lint triggers in #7638 to make this PR smaller

Closes: #7569
bors added a commit that referenced this pull request Nov 11, 2021
New lint `index_refutable_slice` to avoid slice indexing

A new lint to check for slices that could be deconstructed to avoid indexing. This lint should hopefully prevent some panics in other projects and ICEs for us. See #7569 for an example

The implementation specifically checks for immutable bindings in `if let` statements to slices and arrays. Then it checks if these bindings are only used for value access using indices and that these indices are lower than the configured limit. I did my best to keep the implementation small, however the check was sadly quite complex. Now it's around 300 lines for the implementation and the rest are test.

---

Optional future improvements:
* Check for these instances also in `match` statements
* Check for mutable slice bindings that could also be destructed

---

changelog: New lint [`index_refutable_slice`]

I've already fixed a bunch of lint triggers in #7638 to make this PR smaller

Closes: #7569
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.

4 participants