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

De-~[] std::path #13481

Merged
merged 3 commits into from
Apr 14, 2014
Merged

De-~[] std::path #13481

merged 3 commits into from
Apr 14, 2014

Conversation

huonw
Copy link
Member

@huonw huonw commented Apr 12, 2014

Remove the use of ~[] from Path's internals.

huonw added 3 commits April 12, 2014 22:50
@huonw
std: migrate path::unix to using Vec internally.
28e3340
@huonw
std: migrate path::windows to using StrBuf internally.
1283caa 
Same representation change performed with path::unix.

This also implements BytesContainer for StrBuf & adds an (unsafe) method
for viewing & mutating the raw byte vector of a StrBuf.
@huonw
std: update & de-~[] path's tests.
31074fd
@huonw
Copy link
Member Author

huonw commented Apr 12, 2014

(I'd forgot to do std::path::windows. Fixed now.)

hatahet
hatahet reviewed Apr 13, 2014
View reviewed changes
src/libstd/path/posix.rs
@@ -231,7 +237,7 @@ impl GenericPath for Path {

#[inline]
fn is_absolute(&self) -> bool {
self.repr[0] == SEP_BYTE
*self.repr.get(0) == SEP_BYTE
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there going to be syntax sugar (read overloading operator[]) for Vec types?

Copy link
Member Author

Choose a reason for hiding this comment

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

when #6515 is fixed.

@hatahet
Copy link
Contributor

hatahet commented Apr 13, 2014

Sorry for the spam. So is the consensus now to use Vec<T> everywhere as a return type in the standard library instead of ~[T]? Or is this only temporary until DST ~[T] is implemented? Are there no effects on performance as well?

@huonw
Copy link
Member Author

huonw commented Apr 13, 2014

For types like Path which resize themselves internally (e.g. with a .push), Vec is the 100% correct choice, even with DST, since DST's ~[] will not be efficiently resizable.

There has not yet been a decision on library functions returning ~[] or Vec. (I fully expect that returning Vec will actually be the fastest choice.)

@hatahet
Copy link
Contributor

hatahet commented Apr 13, 2014

Thanks for the insight. :)

bors added a commit that referenced this pull request Apr 14, 2014
@bors
auto merge of #13481 : huonw/rust/devec-path, r=alexcrichton
5dd94d8 
Remove the use of ~[] from Path's internals.
@bors bors closed this Apr 14, 2014
@bors bors merged commit 31074fd into rust-lang:master Apr 14, 2014
@huonw huonw deleted the devec-path branch June 27, 2014 06:47
Dylan-DPC pushed a commit to Dylan-DPC/rust that referenced this pull request Nov 1, 2022
@bors
Auto merge of rust-lang#13484 - lnicola:position-encoding, r=lnicola
0340b51 
Switch to upstream `positionEncoding`

Closes rust-lang#13481

This drops support for the custom extension, but that's probably fine.

Draft because it's not tested yet.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 10, 2024
@bors @flip1995
Auto merge of rust-lang#13464 - y21:issue13458, r=flip1995
a21a9fe 
Don't warn on proc macro generated code in `needless_return`

Fixes rust-lang#13458
Fixes rust-lang#13457
Fixes rust-lang#13467
Fixes rust-lang#13479
Fixes rust-lang#13481
Fixes rust-lang#13526
Fixes rust-lang#13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
rust-cloud-vms bot pushed a commit to liwagu/rust that referenced this pull request Oct 10, 2024
@bors @liwagu
Auto merge of rust-lang#13464 - y21:issue13458, r=flip1995
9472047 
Don't warn on proc macro generated code in `needless_return`

Fixes rust-lang#13458
Fixes rust-lang#13457
Fixes rust-lang#13467
Fixes rust-lang#13479
Fixes rust-lang#13481
Fixes rust-lang#13526
Fixes rust-lang#13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 18, 2024
@bors
Auto merge of rust-lang#13464 - y21:issue13458, r=flip1995
8e60f14 
Don't warn on proc macro generated code in `needless_return`

Fixes rust-lang#13458
Fixes rust-lang#13457
Fixes rust-lang#13467
Fixes rust-lang#13479
Fixes rust-lang#13481
Fixes rust-lang#13526
Fixes rust-lang#13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 18, 2024
@bors @flip1995
Auto merge of rust-lang#13464 - y21:issue13458, r=flip1995
806a9df 
Don't warn on proc macro generated code in `needless_return`

Fixes rust-lang#13458
Fixes rust-lang#13457
Fixes rust-lang#13467
Fixes rust-lang#13479
Fixes rust-lang#13481
Fixes rust-lang#13526
Fixes rust-lang#13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@huonw @hatahet @alexcrichton @bors