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

Vec/Str need an efficient way to shift() multi-items #12884

Closed
liigo opened this issue Mar 14, 2014 · 8 comments
Closed

Vec/Str need an efficient way to shift() multi-items #12884

liigo opened this issue Mar 14, 2014 · 8 comments

Comments

@liigo
Copy link
Contributor

liigo commented Mar 14, 2014

to avoid copy/alloc/realloc memory.

in C or unsafe code, you just need to offset the data pointer and len-=n.

@thestinger
Copy link
Contributor

The ring buffer type offers O(1) indexing, O(1) pop from both ends and O(1) amortized append to both ends so I don't think there should be many uses of a vector this way.

The vector type is as good as it's going to get without it becoming another data structure. It just makes sure there's reserved space (much like push) and then shifts it over via our equivalent to memmove.

@liigo
Copy link
Contributor Author

liigo commented Mar 15, 2014

shift() is very bad in performance, and it removes only one item at a time. if you shift() multiple times, ...

@thestinger thestinger reopened this Mar 15, 2014
@thestinger
Copy link
Contributor

Ah, so you want a method to shift more than one at once. It might be useful, although I still think we should be encouraging use of ring buffers :P.

@huonw
Copy link
Member

huonw commented Mar 15, 2014

This could be remove_many(&mut self, starting_index: uint, number_to_remove: uint), and then shifting 10 would be .remove_many(0, 10) (I personally think this is rare enough, and should generally be discouraged in favour of ring buffers, that we don't need a sugary .shift_many(10)).

@lifthrasiir
Copy link
Contributor

@huonw More generally, how about splice(&mut self, start: uint, end: uint, replacements: &[T]) and splice_move(&mut self, start: uint, end: uint, replacements: Vec<T>)? It is the most general form of array manipulation, so having splice methods and nothing else seems to be sufficient.

@liigo
Copy link
Contributor Author

liigo commented Mar 17, 2014

in the PR #12976, I can't find an efficient/safe way to remove the utf-8 BOM header from source string, to avoid copy/alloc/realloc-ing memory.

@liigo liigo changed the title Vec/Str need an efficient way to delete items from left Vec/Str need an efficient way to shift() multi-items Apr 13, 2014
@steveklabnik
Copy link
Member

Today, changes like this would require an RFC, so if you're interested in proposing these methods, you should write one up. :)

@bluss
Copy link
Member

bluss commented Apr 30, 2015

Vec::drain now provides this! As soon as a snapshot includes it, we can solve the fixme in the code with this issue's tag using String::drain.

matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Aug 2, 2022
Use large stack on expander thread

I have verified that this fixes rust-lang#12884 for me.

Hat tip to `@bjorn3` for identifying the cause of the issue.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 13, 2024
Only run `suboptimal_flops` on inherent method calls

Fixes rust-lang#12881

`suboptimal_flops` was making the wrong assumption that a `.log()` method call on a float literal must choose the inherent log method that will always have an argument present (in which case `args[0]` indexing would be fine), but that wasn't the case in the linked issue because at the point of the method call, the exact float type hadn't been inferred yet (and method selection can't select inherent methods when the exact float type has not yet been inferred, in which case it falls back to looking for trait impls and chooses the one that didn't have any parameters).

This fixes it by actually making sure it's a call to an inherent method (could also fix the linked ICE by simply using fallibly indexing via `.get()`, but this felt like it'd fix the root cause: even if there were one argument, it would still be wrong to emit a warning there because it's not the `log` method the lint was expecting). I'm not sure if we need that extra function be in `clippy_utils` but it feels like it could be useful.

changelog: Fixes an ICE in [`suboptimal_flops`]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants