Skip to content

cleanup: replace as[_mut]_slice() calls with deref coercions/slicing syntax #21916

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
Merged
merged 1 commit into from
Feb 5, 2015
Merged

cleanup: replace as[_mut]_slice() calls with deref coercions/slicing syntax #21916

merged 1 commit into from
Feb 5, 2015

Conversation

japaric
Copy link
Member

@japaric japaric commented Feb 3, 2015

@japaric
Copy link
Member Author

japaric commented Feb 3, 2015

@bors: try 82e4655

@bors
Copy link
Collaborator

bors commented Feb 3, 2015

⌛ Testing commit 82e4655 with merge d58e26c...

@gkoz
Copy link
Contributor

gkoz commented Feb 4, 2015

In some cases x.as_slice() is replaced with &x and in others with &x[]. is that supposed to be so? It seemed to me that the latter syntax is redundant.

@alexcrichton alexcrichton reopened this Feb 4, 2015
@bluss
Copy link
Member

bluss commented Feb 4, 2015

FYI &expr[] will be deprecated soon, after .. is added and &expr[..] is the new way (or using deref coercions). See #21879

@japaric
Copy link
Member Author

japaric commented Feb 4, 2015

@gkoz you can't use deref coercions everywhere, one example is passing &Vec<T> where U: Bound is expected even if &[T] implements Bound. It also doesn't work with match &x when the patterns are string literals or slice patterns.

@bluss I have used deref coercions wherever it was possible, and used slicing syntax where it was not. I can change the &x[]s back to as_slice()s, if that helps you with the new .. syntax. I count 127 occurrences in my patch, but I bet there hundreds more in the repo :P.

Alternatively, I could change the &x[] to &*x. (FWIW, I personally prefer &*x over &x[..], but for some reason other rust devs find &**x unsightly)

@alexcrichton
Copy link
Member

I'd be fine switching to &*x in the meantime to prevent churn when switching to [..].

@alexcrichton
Copy link
Member

Ah also, basically r=me when tests are passing, this is one beautiful patch :)

@alexcrichton alexcrichton mentioned this pull request Feb 4, 2015
Merged
@eddyb
Copy link
Member

eddyb commented Feb 4, 2015

@japaric maybe type ascription will solve that problem - seeing a type, even something "skeletal" like &[_], seems more helpful IMO than as_slice or other actions.

@japaric japaric force-pushed the no-as-slice branch 2 times, most recently from 9ceacd8 to 0504f24 Compare February 4, 2015 22:33
@japaric
Copy link
Member Author

japaric commented Feb 4, 2015

@bors: r=alexcrichton 0504f24

@japaric
Copy link
Member Author

japaric commented Feb 4, 2015

@bors: @alexcrichton 116edc1

@japaric
Copy link
Member Author

japaric commented Feb 4, 2015

@bors: r=alexcrichton 116edc1

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

⌛ Testing commit 116edc1 with merge 9b068ce...

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

💔 Test failed - auto-mac-32-opt

@japaric
Copy link
Member Author

japaric commented Feb 5, 2015

@bors: r=alexcrichton a7ccda8

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

⌛ Testing commit a7ccda8 with merge 0512ff1...

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

💔 Test failed - auto-win-32-opt

@japaric
Copy link
Member Author

japaric commented Feb 5, 2015

@bors: r=alexcrichton bfef86a

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

⌛ Testing commit bfef86a with merge 4f35ec7...

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

💔 Test failed - auto-linux-64-x-android-t

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

💔 Test failed - auto-mac-64-nopt-t

@japaric
Copy link
Member Author

japaric commented Feb 5, 2015

@bors: r=alexcrichton 82c5668

@japaric
Copy link
Member Author

japaric commented Feb 5, 2015

@bors: r=alexcrichton 17bc7d8

@bors
Copy link
Collaborator

bors commented Feb 5, 2015

⌛ Testing commit 17bc7d8 with merge 189930f...

bors added a commit that referenced this pull request Feb 5, 2015
@bors
Auto merge of #21916 - japaric:no-as-slice, r=alexcrichton
189930f 
r? @alexcrichton 
cc @eddyb
@bors
Copy link
Collaborator

bors commented Feb 5, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

@bors bors merged commit 17bc7d8 into rust-lang:master Feb 5, 2015
@japaric japaric deleted the no-as-slice branch February 6, 2015 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@japaric @bors @gkoz @bluss @alexcrichton @eddyb