Make IntoIterator lifetime bounds of &BTreeMap match with &HashMap

[canova]

This is a pretty small change on the lifetime bounds of IntoIterator implementations of both &BTreeMap and &mut BTreeMap. This is loosening the lifetime bounds, so more code should be accepted with this PR. This is lifetime bounds will still be implicit since we have type Item = (&'a K, &'a V); in the implementation. This change will make the HashMap and BTreeMap share the same signature, so we can share the same function/trait with both HashMap and BTreeMap in the code.

Fixes #74034.
r? @dtolnay hey, I was touching this file on my previous PR and wanted to fix this on the way. Would you mind taking a look at this, or redirecting it if you are busy?

[pickfire]

Looks good to me.

[nbdd0121]

I don't quite understand the reason why K: 'a, V: 'a would make a difference here, isn't this implied because that's needed for &'a BTreeMap<K, V> to be well-formed?

[canova]

Yeah, 'a lifetime should be implied for K and V, since also we have (&'a K, &'a V) as the Item type. But apparently it does make a difference when we put that lifetime bound to the IntoIterator impl signature. You can see that the test I added doesn't compile without this patch in this playground.

[nbdd0121]

What I am suggesting is that this may be an issue with the compiler and we might want to fix that instead.

[canova]

Well, yes, we can investigate more if we think this is a bug, but I don't think so really. I'm looking all the use cases of IntoIterator implementations in the rust codebase and there is no other implementation that actually adds lifetime bounds to type params: https://dxr.mozilla.org/rust/search?q=%22%3E+IntoIterator+for+%26%27%22&redirect=false
This doesn't look like the correct way of using this trait. Also, I don't see a reason not to land this change, because it's better to make the signatures of HashMap and BTreeMap match each other.

[dtolnay]

Thanks!

[dtolnay]

@bors r+

[bors]

📌 Commit cedf96c has been approved by dtolnay

[nbdd0121]

Well, yes, we can investigate more if we think this is a bug, but I don't think so really.

This is certainly a bug. If you look at the minimal example in the original issue:

fn foo<'a, K, V, I: 'a + IntoIterator<Item = (&'a K, &'a V)>>(i: I) {}

fn bar() {
    let map = HashMap::<u32, u32>::new();
    foo(&map);
    
    let map = BTreeMap::<u32, u32>::new();
    foo(&map);
}

This code is rejected in Rust 2015 for both HashMap and BTreeMap, suggesting that K: 'a and V: 'a must be added. In Rust 2018 this is now implied, so it should accept BTreeMap, regardless that bound is specified implicitly or explicitly. I have no objection to this PR, but I suggest we keep the original issue open or open a new issue after this PR is merged to track this discrepancy.

[canova]

This code is rejected in Rust 2015 for both HashMap and BTreeMap, suggesting that K: 'a and V: 'a must be added. In Rust 2018 this is now implied, so it should accept BTreeMap, regardless that bound is specified implicitly or explicitly. I have no objection to this PR, but I suggest we keep the original issue open or open a new issue after this PR is merged to track this discrepancy.

Hm, this minimal example also fails for HashMap in version 2018 and 2015. So I assume this is not exactly the bug issue tried explain. But you're right, there is something wrong with the example and should be fixed. I can't reopen the previous issue, so I opened #75252 to track this instead. Thanks for the explanation!