Future-prove collections::VecMap
#20150
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
#20148 would be really helpful here. |
|
Want to think about this some more. CC me. |
|
@gankro The same should be done for |
|
|
|
@gankro Have you thought about this? |
|
@tbu- Sorry, I was planning to work all this week but decided at the last minute that taking a one-week vacation would probably be better for my sanity. Going through some discussion back-logs today, will get back to proper reviewing ~monday. |
|
If we simply use |
| v: Vec<Option<V>>, | ||
| invariant_type: InvariantType<K>, |
There was a problem hiding this comment.
Why InvariantType rather than CovariantType?
There was a problem hiding this comment.
Because I don't understand the exact semantics and only know that InvariantType is the strictest. CovariantType would be more correct here?
|
@gankro it seems a little strange to have that order of type params, but it does seem like something that might be back-compat? |
| } | ||
|
|
||
| // Tell inference to not infer `uint`. | ||
| struct _Nothing; |
There was a problem hiding this comment.
I'm not familiar with this trick, can you elaborate?
There was a problem hiding this comment.
Without this struct, inference will assume that uint is the only thing that implements VecMapKey, and will infer the key type to uint, which would make making VecMap truly generic over its key type a backward-incompatible change.
|
I'm torn. This feels like the right thing to do, but also a bit over-engineer-y. But also I want to make it more over-engineer-y. :S |
|
@gankro I'm not intending this to be the actual solution, this should just provide possible backward-compatiblity when making To furtherly prevent people from implementing this trait, we could add add a function signature to the trait that is impossible to achieve in safe code, like |
|
@tbu- Sorry for taking so long to weigh in on this. Do you mind holding off until after we (1) fully resolve the int debate and (2) ship alpha next week? We don't plan to stabilize |
|
@aturon I have no problem with this, but keep in mind that this affects other |
|
@tbu- Good point :-) Regardless, we'll need to settle this question during the alpha cycle. |
Since `VecMap` might want to have key type parameter at some point, add a key type parameter now, so that code running today will still run when actual generalization of the key is added. The trait `VecMapKey` should really be private, but this is blocked by the fact that you cannot export a struct, function or trait implementation with one of the type parameters being bound by a private trait.
tbu-
force-pushed
the
pr_collections_key
branch
from
66465f0 to
2e6285e
Compare
Since
VecMapmight want to have key type parameter at some point, add a keytype parameter now, so that code running today will still run when actual
generalization of the key is added.
The trait
VecMapKeyshould really be private, but this is blocked by the factthat you cannot export a struct, function or trait implementation with one of
the type parameters being bound by a private trait.