Conversation
src/libcollections/binary_heap.rs
Outdated
There was a problem hiding this comment.
Due to the way stability works these instability annotations unfortunately won't be read and considered. In this case that's ok, though, but could you change these to #[stable(feature = "extend_ref", since = "1.2.0")]?
|
🎊 r=me with @alexcrichton's nits addressed |
|
Sorry for the delay! I modified the stability markers and removed |
|
@bors r+ |
|
📌 Commit e2dd771 has been approved by |
|
⌛ Testing commit e2dd771 with merge 0246d8d... |
|
💔 Test failed - auto-linux-64-x-android-t |
|
Closes rust-lang#25976.
|
Oh, sorry, I fixed it. |
|
@bors r+ No worries! |
|
📌 Commit b36ed7d has been approved by |
I had to use `impl<'a, V: Copy> Extend<(usize, &'a V)> for VecMap<V>` instead of `impl<'a, V: Copy> Extend<(&'a usize, &'a V)> for VecMap<V>` as that's what is needed for doing ```rust let mut a = VecMap::new(); let b = VecMap::new(); b.insert(1, "foo"); a.extend(&b) ``` I can squash the commits after review. r? @gankro
|
Such inference failures are interesting & important to note since this will be a minor breaking change in that case. (Which is permitted). Thanks for implementing this!! |
|
@brson Do we need to track this? This caused a very minor change to how Extend-using functions infer. In this case seen when using |
brson
added
regression-from-stable-to-nightly
|
@bluss If it's a regression, yeah, I suppose so. Also nominating for beta for the same reason. |
|
@bluss I didn't spell out that in the RFC because I guess it seemed obvious: every generalization of an API can break someone's code in inference. |
alexcrichton
added
the
T-libs-api
|
We discussed this in triage today and decided to not backport this, we're going to have minor regressions like this and we're probably not going to aggressively backport them, they'll make their way into the stable channel eventually. |
alexcrichton
removed
the
beta-nominated
It appears that these impls were left out of #25989 by mistake. r? @alexcrichton I'm not sure what the stability markers for these should be.
6 participants
I had to use
impl<'a, V: Copy> Extend<(usize, &'a V)> for VecMap<V>instead ofimpl<'a, V: Copy> Extend<(&'a usize, &'a V)> for VecMap<V>as that's what is needed for doingI can squash the commits after review.
r? @gankro