SliceExt is half-unusable and a semver hazard

[steffahn]

Context

The issues around extension traits that model future (unstable) std features [or the other way around, who are so popular that std wants to uplift parts of their API] isn’t unknown.

Extension traits of traits like Itertools regularly create issues due to the method-resolution breakage that comes if Iterator stabilizes one of their methods. There is ongoing work to improve the situation by adding a fallback rule to disambiguate.

For extension traits of concrete types, the situation is already a lot better. We already have a fallback rule: the concrete type’s method wins! Users will not run into ambiguity errors when e.g. the to_vec_in method is stabilized eventually. HOWEVER such stabilization will change the result of resolution; and the type signatures don’t match!

Main issue 1 (semver hazard)

In other words, stable users of allocator-api2 (which are the most important users; let’s not break stable users!) will be able to use SliceExt today, e.g.

use allocator_api2::{alloc as a, vec as v, SliceExt};

fn main() {
    let v: v::Vec<u8> = [1, 2, 3].to_vec_in(a::Global);
    // ...
}

but when to_vec_in is ever stabilized, this usage breaks, because v::Vec and std::vec::Vec will still be distinct types. [They have to stay distinct especially if the final allocator-related API for std::vec::Vec ends up changed semver-incompatibly from the current state of allocator_api2::vec::Vec.]

Main issue 2 (already half-unusable)

Additionally, to_vec and repeat are already broken, you can’t call them with ordinary method call syntax, even today.

And using to_vec_in isn’t any fun either. The compiler (thankfully) correctly identifies the fundamental issue here already and warns all users:

warning: a method with this name may be added to the standard library in the future
 --> src/main.rs:4:35
  |
4 |     let v: v::Vec<u8> = [1, 2, 3].to_vec_in(a::Global);
  |                                   ^^^^^^^^^
  |
  = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior!
  = note: for more information, see issue #48919 <https://github.com/rust-lang/rust/issues/48919>
  = help: call with fully qualified syntax `to_vec_in(...)` to keep using the current method
  = note: `#[warn(unstable_name_collisions)]` on by default
help: add `#![feature(allocator_api)]` to the crate attributes to enable `slice::<impl [T]>::to_vec_in`
  |
1 + #![feature(allocator_api)]
  |

Fix

My proposed fix is to only offer SliceExt methods with distinct names from the standard ones. For example the 2 suffix that the crate name already uses could be used here, forming method names like

fn to_vec_in2<A: Allocator>(&self, alloc: A) -> Vec<T, A>
    where T: Clone;

fn repeat2(&self, n: usize) -> Vec<T, Global>
    where T: Copy;

fn to_vec2(&self) -> Vec<T, Global>
    where T: Clone;

or alternatively like

fn to_vec_in_2<A: Allocator>(&self, alloc: A) -> Vec<T, A>
    where T: Clone;

fn repeat_2(&self, n: usize) -> Vec<T, Global>
    where T: Copy;

fn to_vec_2(&self) -> Vec<T, Global>
    where T: Clone;

While already making breaking changes to SliceExt, one could also consider adding a : Sealed restriction pattern to it, so that any future addition of new API is actually non-semver-breaking.

[steffahn]

One thing I hadn’t thought about is that allocator-api2 should perhaps avoid unnecessary major version bumps (e.g. 0.2.* to 0.3.*) since users would generally be using it as a public dependency.¹

So regarding non-breaking changes, alternatively-named variants could simply be added to the existing trait.

Footnotes

1. At least Allocator itself should best not be broken. If major version bumps are becoming necessary eventually, there’s still the semver trick of re-exporting the updated version of it, while still being able to at least replace something like Vec, Box, and so on. ↩