Skip to content

add #[experimental] as_string/as_vec functions #17447

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

Merged
merged 1 commit into from
Oct 8, 2014
Merged

add #[experimental] as_string/as_vec functions #17447

merged 1 commit into from
Oct 8, 2014

Conversation

thestinger
Copy link
Contributor

This provides a way to pass &[T] to functions taking &U where U is
a Vec<T>. This is useful in many cases not covered by the Equiv trait
or methods like find_with on TreeMap.

@thestinger
Copy link
Contributor Author

GitHub wouldn't let me reopen #16713.

@thestinger thestinger added I-slow Issue: Problems and improvements with respect to performance of generated code. A-libs labels Sep 24, 2014
#[unsafe_destructor]
impl<'a, T> Drop for AsVec<'a, T> {
fn drop(&mut self) {
self.x.len = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

A more forward-compatible way of doing this might be

mem::forget(mem::replace(self, Vec::new())).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would work but it won't necessary be optimized out. This will be forwards compatible because the basic properties of strings / vectors are going to end up as guarantees for unsafe code. It's defined in the same module anyway.

@aturon
Copy link
Member

aturon commented Oct 1, 2014

First, sorry for the delay in decision about this PR, which was partly due to uncertainty around the overall strategy for improving equiv.

We're now ready to move forward with this API. Although collections reform offers a somewhat more ergonomic and general way to improve equiv and friends (through the Borrow trait), there will undoubtedly still be situations where an API uses generics but does not opt in to Borrow. The APIs in this PR give you a slick way to work around that problem.

Thanks for discovering and contributing this clever hack!

@aturon
Copy link
Member

aturon commented Oct 1, 2014

A couple of questions:

  • Would you consider moving all of this code into the str and slice modules? Over time, functionality that operates on str and [T] types has migrated into these modules.
  • What was the rationale behind making these free functions rather than methods? Just that they're not intended for common use?
  • I worry a bit about the type names AsString and AsVec, which sounds more general than they are and which may conflict with trait names we'd like to add in the future. Admittedly, there's not an obviously better choice, but maybe worth some thought.

@thestinger
Copy link
Contributor Author

  • Would you consider moving all of this code into the str and slice modules? Over time, functionality that operates on str and [T] types has migrated into these modules.

That would be fine, although I think they're more tied to String and Vec<T>.

  • What was the rationale behind making these free functions rather than methods? Just that they're not intended for common use?

It didn't feel right to associate them with [T] and str as methods but I guess that would be fine.

  • I worry a bit about the type names AsString and AsVec, which sounds more general than they are and which may conflict with trait names we'd like to add in the future. Admittedly, there's not an obviously better choice, but maybe worth some thought.

The type names could remain experimental even if the method is marked stable, since I doubt there are many cases where they need to be referred to.

@aturon
Copy link
Member

aturon commented Oct 1, 2014

@thestinger Ok, thinking more about it, leaving these as free functions is probably for the best, but I do think they make sense to go in the slice modules, since the main "receiver" types are slice types.

Also, what about DerefString and DerefVec for the type names? I agree that the name isn't important for stability purposes, but with core modules people tend to look a lot at the documentation and a name like AsString is likely to send the wrong message.

r=me with the above.

This provides a way to pass `&[T]` to functions taking `&U` where `U` is
a `Vec<T>`. This is useful in many cases not covered by the Equiv trait
or methods like `find_with` on TreeMap.
bors added a commit that referenced this pull request Oct 8, 2014
This provides a way to pass `&[T]` to functions taking `&U` where `U` is
a `Vec<T>`. This is useful in many cases not covered by the Equiv trait
or methods like `find_with` on TreeMap.
@bors bors closed this Oct 8, 2014
@bors bors merged commit f744479 into rust-lang:master Oct 8, 2014
@thestinger thestinger deleted the silly_string branch October 13, 2014 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants