Vec::recycle (experimental API implementation) #66069
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
jonas-schievink
added
A-collections
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
It seems that a formatting check failed. I'm gonna reformat this later. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Centril
reviewed
Nov 4, 2019
src/liballoc/vec.rs
Outdated
| #[unstable(feature = "recycle_vec", reason = "new API", issue = "0")] | ||
| pub fn recycle<U>(mut self) -> Vec<U> { | ||
| self.truncate(0); | ||
| // TODO make these const asserts once it becomes possible |
There was a problem hiding this comment.
Is there an actual plan for making this possible? Even with compile time assertions, you wouldn't actually be able to reference T and U inside e.g. a const item defined in this body.
There was a problem hiding this comment.
That is a question that needs to be resolved. It is mentioned as an unresolved question in the RFC. The problem is that changing to compile-time error is a breaking change, so whatever gets decided, we must commit to before stabilizing.
I assumed that once the const functions support conditionals, creating compile time assertion for this case would be possible, but I had a hidden assumption that if there is a generic const function, I could pass it generic types that are in scope. Is the latter not the case? I wonder if there are explicit plans about supporting or not supporting that, or is it just more about how it currently is?
There was a problem hiding this comment.
That's not the case and there are no current plans to change that so don't expect it to change within the next 1.5 years at least.
There was a problem hiding this comment.
All right, I removed the FIXME. I guess it seems good enough just to panic, although I'd love to have a compile-time guarantee, and I think I'm not alone. I wonder if there is any other avenue of attack that would accomplish this.
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I'm getting a CI error like this: I wonder if 3000 is the threshold value and this PR just happened to break it? Even if |
|
You can add the ignore directive but it would be good if you could follow up with the splitting. |
|
I'll starting working on one. That's actually a nice, manageable thing to do for us rustc noobs :) Adding the ignore directive for now. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Ping from triage: Thanks! |
|
I am going to update the unresolved questions of this RFC. I’ll have time to do that on Monday or Tuesday next week. Edit: oops, responded without checking which thread this was. Never mind. |
|
Is this worth reviewing while the RFC is still being discussed? |
Dylan-DPC-zz
added
S-blocked
|
Marking this as blocked because it is blocked on RFC |
Centril
reviewed
Nov 15, 2019
| /// # } | ||
| /// ``` | ||
| /// | ||
| /// # Note about stabilization |
|
☔ The latest upstream changes (presumably #67917) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably #68142) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closing this as this is blocked on the RFC and the RFC will take a while |
Dylan-DPC-zz
added
S-blocked-closed
and removed
S-blocked
jyn514
added
S-blocked
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements the API described here rust-lang/rfcs#2802 behind a feature flag
recycle_vec.