rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.35k stars 12.72k forks source link

improve documentation for AsRef/AsMut #62586

Open BurntSushi opened 5 years ago

BurntSushi commented 5 years ago

AsRef/AsMut is fairly common source of changes that result in breaking others' code. Most such changes occur by adding additional AsRef/AsMut impls, which in turn break type inference that users are unwittingly relying on. According to Rust's API evolution, this is, strictly speaking, allowable breakage.

Almost all such instances of code breakage correspond to a misuse of AsRef/AsMut, by using it situations where there are no constraints to help type inference. Idiomatic use of these traits is typically by specifying them in a context where the target type is known. For example:

fn foo<T: AsMut<[u8]>>(something: T) {
    // this will never fail, because the target type is unambiguous
    let my_mutable_slice = something.as_mut();
    // ...
}

Code like this is robust with respect to additional implementations of AsMut.

This issue is about improving the documentation of AsRef/AsMut to cover these important caveats. Currently, the docs include no mention of what incorrect usage actually is, or the fact that incorrectly using this trait can result in a new Rust compiler failing to compiler your code. (There is an example of correct usage, but no broader discussion of the intricacies of using this trait.) Additionally, the docs, IMO, don't do a good enough job differentiating AsRef from Borrow. The docs mention some specific differences, but don't cover high level concerns. It's very difficult, even for me, to take the noted differences in the docs and extrapolate that to a heuristic for when I should prefer one over the other.

This issue is motivated by the fallout from #60958, and my own recent experience in dealing with this in https://github.com/BurntSushi/rust-csv/issues/160 (which is particularly subtle).

lnicola commented 5 years ago

Why doesn't the compiler tell why the call is ambiguous? It could enumerate the implementations that as_ref() could come from.

BurntSushi commented 1 year ago

Another example of misuse that lead to a new impl breaking (significant) downstream consumers: https://github.com/BurntSushi/bstr/issues/149

Basically, I added a impl AsRef<BStr> for BStr, but folks were using bstr.as_ref() in a non-generic context that relied on inferring the target type to be &[u8]. But with the new impl, the target type became ambiguous and inference failed.

Stargateur commented 1 year ago

What are we waiting for?