Type inference breakage in 0.7.5 on ArrayString::as_ref

[jannic]

This code compiles with arrayvec 0.7.4:

use arrayvec::ArrayString;

fn main() {
    let array_string = ArrayString::<32>::new();
    let as_ref = array_string.as_ref();
    let _bytes = as_ref.as_bytes();
}

However, with arrayvec 0.7.5 and 0.7.6, I get the following error:

error[E0282]: type annotations needed for `&_`
 --> src/main.rs:5:9
  |
5 |     let as_ref = array_string.as_ref();
  |         ^^^^^^
6 |     let _bytes = as_ref.as_bytes();
  |                         -------- type must be known at this point
  |
help: consider giving `as_ref` an explicit type, where the type for type parameter `T` is specified
  |
5 |     let as_ref: &T = array_string.as_ref();
  |               ++++

This is because there are now two AsRef impls, namely:

impl<const CAP: usize> AsRef<str> for ArrayString<CAP>
{
    fn as_ref(&self) -> &str { self }
}

impl<const CAP: usize> AsRef<Path> for ArrayString<CAP> {
    fn as_ref(&self) -> &Path {
        self.as_str().as_ref()
    }
}

Before, only the first existed, so the compiler could infer that as_ref must be a &str. But with the second implementation, it became ambiguous, and now the compiler needs a type annotation.

I'm not sure how to best handle this:

• reverting the change would hurt users of the new implementation
• yanking 0.7.5 and 0.7.6 and releasing it as 0.8.0 also feels like a big hassle for this small change

As the fix for affected users is quite simple (add a type annotation), it may be best to just document this change?

[bluss]

Another example is that breaking type inference is not considered major.

This is because it's possible to avoid being broken by such a change by adding explicit type annotations in downstream code. In principle, better tooling should be able to add these kinds of type annotations when they become necessary. In the future, this change might no longer be breaking — so it's a reasonable choice to make it non-major today.

https://predr.ag/blog/semver-in-rust-tooling-breakage-and-edge-cases/

It is not generally considered breaking to add trait implementations. Rust is designed for this additivity.

As a general rule AsRef is a conversion trait and users should prefer to use it only for constrained conversions, not open code it: for those cases use the (many) alternatives that are not ambiguous in type.

[jannic]

Yes, I fully agree. (And I was live at FOSDEM to see Predrag give that great talk!)

Breaking changes are not black/white. Calling every instance where type inference may break a major change in the semver sense would not be viable.

Still, in this case, it did break a build and it took me more than an hour to fully understand what was going on. (Didn't happen in my own code, but I was trying to build a debian package of some software that uses arrayvec as a dependency. The build failure happened deep within a build pipeline, in a container with a different debian version installed, etc pp.)

And be assured that I'm not complaining. All I want is to raise a bit of awareness, so maybe the next person with the same issue can find the solution a little bit faster.

That's why me report concluded with "it may be best to just document this change". And to be honest, I'm not even sure about the best place for such documentation, otherwise I would have proposed some specific change.

Perhaps you could add "This may break type inference if you use as_ref without type annotations." to the release notes of 0.7.5?

Anyway, this is a minor issue. Thanks for your work on arrayvec, it's a nice library!

(EDIT: And it's also fine if you just close this report without any change!)