Breaking change in 1.1.0 release

[ctz]

What version of the csv crate are you using?

1.1.1

Briefly describe the question, bug or feature request.

I use this crate through three levels of indirection, and recently my crate stopped building. I bisected this to the 1.0.7 -> 1.1.0 change on this crate.

Include a complete program demonstrating a problem.

Minimal test case:

$ cargo new test1
$ cd test1/
$ echo 'csv = "1"' >> Cargo.toml
$ cat src/main.rs 
fn main() {
    println!("Hello, world!");
}

(... edit such that ...)

$ cat src/main.rs
use csv as _;

fn main() {
    let mut buf = [0u8; 8192];
    let _ = &mut buf.as_mut();
    println!("Hello, world!");
}

$ cargo build

Changing the dependency version to "=1.0.7" and everything builds OK.

What is the observed behavior of the code above?

error[E0283]: type annotations required: cannot resolve `[u8]: std::convert::AsMut<_>`
 --> src/main.rs:5:22
  |
5 |     let _ = &mut buf.as_mut();
  |                      ^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0283`.

What is the expected or desired behavior of the code above?

Test case crate builds OK as it did with 1.0.7

--

This is frankly quite confusing to me, and surprising. It feels like a bug in the compiler that an extension trait which isn't used or referred to has any effect on type resolution/inference?

[BurntSushi]

Thanks for the report, but I don't think this is a compiler bug, and it is definitely not a breaking change according to the Rust API evolution guidelines. In particular, this sort of inference breakage (among other things) is explicitly allowed in semver compatible releases, because otherwise, it would be impossible to add anything. (Every addition to a library has the potential to break someone's code.)

Moreover, using as_mut (or as_ref) like this outside of a generic context is a misuse. The docs for AsRef/AsMut leave a lot to be desired on this front, but it should generally only be used in contexts where the target type is known. Otherwise, you leave yourself open to these type inference failures. While I understand the example you've given here is minimized, the original code should almost certainly be changed to use a concrete method instead of the generic AsMut. For example, if you have a Vec<T> and you want a &mut [T], then you should either rely on auto-deref (e.g., &mut myvec) or use a concrete method like Vec::as_mut_slice, and not the AsMut trait. You should use the AsMut trait when you're writing a generic type or function in which you want to give more flexibility to the caller, e.g.,

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

Now, the specific reason why the csv 1.1 release introduced a type inference regression here is subtle. But basically, csv brought in a new dependency, bstr, which adds new AsMut implementations for [u8]. This turned something that was once unambiguous into something ambiguous. Specifically, your as_mut here could now return not only a &mut [u8], but also a &mut BStr. Absent any additional context, type inference fails. For example, both of these programs work, while simultaneously demonstrating the ambiguity:

use csv as _;

fn main() {
    let mut buf = [0u8; 8192];
    let _: &mut [u8] = &mut buf.as_mut();
    println!("Hello, world!");
}

use csv as _;

fn main() {
    let mut buf = [0u8; 8192];
    let _: &mut bstr::BStr = &mut buf.as_mut();
    println!("Hello, world!");
}

[BurntSushi]

FWIW, I did not immediately know the answer to this. I had to bisect the commits in csv between 1.0.7 and 1.1.0 against your example program. That led me to the commit that introduced bstr, and from that, I knew bstr provided additional AsMut impls as seen here: https://docs.rs/bstr/0.2.1/bstr/struct.BStr.html (^f and search for AsMut until you see impl AsMut<BStr> for [u8]).

[ctz]

Thanks for the detailed reply and I appreciate the time you spent looking at this.