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.

Sign up for GitHub

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

Jump to bottom

Suggest split_at_mut on multiple mutable index access #68819

Merged
merged 1 commit into from
Feb 5, 2020
Merged

Suggest split_at_mut on multiple mutable index access #68819

merged 1 commit into from
Feb 5, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Feb 4, 2020 •
edited by oli-obk
Loading

cc #58792.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Feb 4, 2020
@estebank
Copy link
Contributor Author

estebank commented Feb 4, 2020

cc @oli-obk a (very) simplified version of the suggestion we discussed last RustConf.

memoryruins
memoryruins reviewed Feb 4, 2020
View reviewed changes
src/test/ui/suggestions/suggest-split-at-mut.stderr
| ------ first borrow later used here
|
= help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices

Copy link
Contributor

Choose a reason for hiding this comment

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

For short arrays, should we hint at patterns too? https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=a9b151b57dc05010be68d1d714be741c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that would be a good advice to give, but the compiler at this time doesn't know the possible size of the vector. I think that can be good follow up work.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2020

Edited main post as to not close the issue yet. Still merging since this makes the split_at_mut methods so much more discoverable.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 4, 2020

📌 Commit 0f73133 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 4, 2020
@petrochenkov
Copy link
Contributor

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned petrochenkov Feb 4, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#68819 - estebank:split_at_mut, r=oli-obk
793a5e6 
Suggest `split_at_mut` on multiple mutable index access

cc rust-lang#58792.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 4, 2020
Merged
bors added a commit that referenced this pull request Feb 5, 2020
@bors
Auto merge of #68831 - Dylan-DPC:rollup-j6x15y9, r=Dylan-DPC
4ff8fb9 
Rollup of 7 pull requests

Successful merges:

 - #68282 (Instrument C / C++ in MemorySanitizer example)
 - #68758 (Fix 59191 - ICE when macro replaces crate root with non-module item)
 - #68805 (bootstrap: fix clippy warnings)
 - #68810 (Remove Copy impl from OnceWith)
 - #68815 (remove redundant imports (clippy::single_component_path_imports))
 - #68818 (fix couple of perf related clippy warnings)
 - #68819 (Suggest `split_at_mut` on multiple mutable index access)

Failed merges:

r? @ghost
@bors bors merged commit 0f73133 into rust-lang:master Feb 5, 2020
@HKalbasi
Copy link
Member

HKalbasi commented Aug 7, 2021

I wonder why this PR works for foo[2] and foo[3] but doesn't work for foo[..2] and foo[2..] (so doesn't solve the #58792)? It seems compiler detects former as borrow of a projection (i.e. foo[_]) but the latter as borrow of foo itself. From my understanding they are two calls of Index trait so they shouldn't be different.

@estebank
Copy link
Contributor Author

estebank commented Aug 8, 2021

@HKalbasi Yes, I am not sure why there's divergence in the behavior. Maybe @oli-obk might know what's going on there.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 9, 2021

usize indexing is a basic MIR operation, so the trait is not involved at all, while slicing requires going through the index trait, and thus you get a simple borrow and then a function call on that reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@memoryruins memoryruins memoryruins left review comments

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@estebank @rust-highfive @oli-obk @bors @petrochenkov @HKalbasi @memoryruins