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

add #[experimental] as_string/as_vec functions #16713

Closed
wants to merge 1 commit into from
Closed

add #[experimental] as_string/as_vec functions #16713

wants to merge 1 commit into from

Conversation

thestinger
Copy link
Contributor

This provides a way to pass &[T] to functions taking &U where U is
a Vec<T>. It can be seen as an alternative to the Equiv trait, as it
handles all of the common cases well, without requiring a new trait and
new equiv methods. Methods like find_with on TreeMap taking a closure
with an equivalent Ord implementation are still useful.

Closes #14549

@thestinger
Copy link
Contributor Author

cc @pcwalton

@alexcrichton
Copy link
Member

I think that the issue referred to in #14549 is a more wide-reaching issue than just Vec and String, and like adding foo_equiv, this seems like a very specific solution to a much more general problem. For example, if the key value in a hash map is Box<T> then I should be able to query with &T instead of just &Box<T>.

I know that @aturon has been working on solutions to this problem with multidispatch and some other RFCs he's got cooking, and I'm curious what his opinion on this is here.

@thestinger
Copy link
Contributor Author

There's already #12135 about Equiv in general. The TreeMap type provides a find_with method taking a closure already, so with a convenient way to handle the common slice case I think that is solved.

@thestinger
Copy link
Contributor Author

This is also useful outside of the context of equality / ordering, as it applies to any generic types exposing an &T method, where T is potentially String or Vec<T>.

@huonw
Copy link
Member

huonw commented Aug 24, 2014

We can use the same trick for viewing a &T as a &Box<T>.

This isn't necessarily general enough to work with all data types, e.g. a particularly fancy rope/fingertree-based String or a StableVec (effectively Vec<Box<T>> to avoid elements moving in memory), but these are rare. It covers the vast majority of uses cases, and certainly closes a small (not memory-safety) hole: equivalent values need to have the same hash.

@Kimundi
Copy link
Member

Kimundi commented Aug 25, 2014

I think this would be worthwhile to have as an experiment, thats what #[experimental] is for after all.

@pcwalton
Copy link
Contributor

I think that we should wait for @aturon's RFC. I'll make sure he's aware of and considers this alternative.

@aturon
Copy link
Member

aturon commented Aug 26, 2014

This is a clever idea, and may be worth providing even if we deal with _equiv in some other way.

For reference, the current proposal for dealing with _equiv methods is in the associated items RFC. Essentially, the proposal is to allow each (owned) key type to provide an associated "query" type. A typical use case is String with associated query type str, but for non-sliceable types you'd have the owned and query types be the same. The benefit is that there's just a single API surface (no _equiv methods) with a pretty straightforward signature. It also means that you can write:

// map: HashMap<String, T>
map.find("some static string")

rather than

map.find("some static string".as_string())

@Florob
Copy link
Contributor

Florob commented Aug 30, 2014

FWIW, I'm currently fighting with a case where I have (String, Option<String>) as the key. I might be mistaken, but I don't think either _equiv() or associated items help me with querying this via (&str, Option<&str>), because I'm not allowed to impl the required traits. This PR however would at least allow me to query without allocating.

@thestinger
Copy link
Contributor Author

Yes, it's a general solution to a class of problems with String/Vec. It helps in a lot more than the Equiv case.

@nikomatsakis
Copy link
Contributor

Dear god what a hack. And I mean that in the best possible way.

I do want to see if we can do something more general than this, but it's interesting to know that it's an option.

@nikomatsakis
Copy link
Contributor

@Florob I don't think this would help in that case, at least not directly. Your key type is the tuple (String, Option<String>), and this only allows you to go from &str to String, not from (&str, Option<&str>) to (String, Option<String>). You could probably write a similar wrapper to achieve it though. That said, I think @aturon's proposed approach also handles your case, and in a rather simpler way: but you would probably have to newtype your tuple.

@thestinger
Copy link
Contributor Author

@nikomatsakis: Well, that's why I marked it #[experimental]. It's very useful today, even if the map key issue is dealt with another way. If there ends up being a better way to do this, removing it won't be a breaking change or even an unexpected change.

This provides a way to pass `&[T]` to functions taking `&U` where `U` is
a `Vec<T>`. It can be seen as an alternative to the `Equiv` trait, as it
handles all of the common cases well, without requiring a new trait and
new equiv methods. Methods like `find_with` on TreeMap taking a closure
with an equivalent `Ord` implementation are still useful.

Closes #14549
@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase!

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

Successfully merging this pull request may close these issues.

Please add find_equiv to TreeMap
8 participants