Breaking changes by adding default method (e.g. read_exact)

[mvdnes]

Recently, the read_exact method was added to the Read trait. See https://github.com/rust-lang/rust/issues/27585 and its implementation in https://github.com/rust-lang/rust/pull/27588.

This introduction breaks my zip crate (mvdnes/zip-rs), as that one uses the read_exact method from my podio crate.

The signatures do not match:

// std::...::Read
fn read_exact(&mut self, mut buf: &mut [u8]) -> Result<()>;`
// podio::ReadPodExt
fn read_exact(&mut self, usize) -> io::Result<Vec<u8>>;

To fix this, I have to introduce a patch as in here, but this means that it would only compile on nightly and no longer on stable.

I was wondering whether this issue is solely my problem, or a larger problem in general. This has some strong issues for stability. I am aware of the API evolution RFC, but was not able to pinpoint the exact case that this falls into.

Also, what is the best course of action to get it to work both on nightly and on stable. Renaming the method is a breaking change to my own API. This is fine for my simple never-used crate, but what about the larger picture. Another solution is to use UFCS in zip-rs, but that is not something I would like to do possibly on every Rust release.

[daboross]

If you change all usages of x.read_exact(size) to ReadPodExt::read_exact(x, size), the code will work on both stable and nightly, won’t change any APIs and won’t be tied to an unstable feature.

This is mentioned in https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-adding-a-defaulted-item:

According to the basic principles of this RFC, such a change is minor: it is always possible to annotate the call t.foo() to be more explicit in advance using UFCS: Trait2::foo(t).

[mvdnes]

Thanks for the suggestion. I was already thinking about something like that.

This does not remove my more fundamental issues with this however, where after Rust 1.0 everything should be working in all of 1.x apart from obvious bugs.

[nikomatsakis]

I was wondering whether this issue is solely my problem, or a larger problem in general. This has some strong issues for stability. I am aware of the API evolution RFC, but was not able to pinpoint the exact case that this falls into.

This is certainly a known problem. We weren’t sure how big a problem it would be in practice, but the scenario you’re describing here is exactly one of the obvious problem vectors. Basically the desire to move a popular extension method into the standard library.

One of the mechanisms that has been tossed around for mitigating this problem (discussed in the RFC, I believe) was having cargo dependencies store an “elaborated” form (that used UFCS). This would still mean that you have to fix your code, but it would mean that people depend on your code via a cargo dependency do not break. Clearly an improvement, if not everything you might want.

The idea of ranking trait methods in some other way has also been considered – or perhaps mitigating the problem by supporting overloading based on arity or some other means – but I think nobody has tried to work this out in much detail, and I am dubious.

[mvdnes]

Thanks for your response! This was more or less what I expected.

I am glad it is on the agenda somewhere.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.