Special case inference around AsRef<Vec<T>> to support existing code without reverting #95098
#96446
Conversation
…e without reverting rust-lang#95098 rust-lang#95098 introduces new `From` impls for `Vec`, which can break existing code that relies on inference when calling `.as_ref()`. This change explicitly carves out a bias in the inference machinery to keep existing code compiling, while still maintaining the new `From` impls. Reported in rust-lang#96074.
rustbot
added
the
T-compiler
|
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I...don't like this. It's clever, but I think a revert #95098 is more prudent. |
|
@jackh726 this was a proof of concept to see what would be necessary to bias the inference machinery to get these to work without disrupting the ecosystem. I was particularly intrigued when it comes to rust-lang/rfcs#3240, as that will require checks similar to this one (not hardcoded, but the same thing). |
| && tcx.is_diagnostic_item(sym::AsRef, other.def_id) | ||
| && tcx.is_diagnostic_item(sym::AsRef, victim.def_id) |
There was a problem hiding this comment.
These would have to be lang_item checks instead (to not lie about AsRef and Vec not being language items after this).
| && other.substs.len() > 1 | ||
| && victim.substs.len() > 1 |
There was a problem hiding this comment.
These are likely redundant with the two lines above.
| && tcx.is_diagnostic_item(sym::Vec, def.did()) | ||
| { | ||
| // If this is an `AsRef` implementation that can go either way for | ||
| // `AsRef<[T]>` or `AsRef<Vec[T]>`, prefer the former. |
There was a problem hiding this comment.
| // `AsRef<[T]>` or `AsRef<Vec[T]>`, prefer the former. | |
| // `AsRef<[T]>` or `AsRef<Vec<T>>`, prefer the former. |
|
r? @estebank |
#95098 introduces new
Fromimpls forVec, which can break existingcode that relies on inference when calling
.as_ref(). This changeexplicitly carves out a bias in the inference machinery to keep
existing code compiling, while still maintaining the new
Fromimpls.Reported in #96074.