sled icon indicating copy to clipboard operation
sled copied to clipboard

Implement From<&[u8; N]> and From<[u8; N]> for all N for IVec

Open xa888s opened this issue 5 years ago • 3 comments

Why

Currently one can only convert arrays up to a certain limit (32) to an IVec, with this change one can convert from arbitrary arrays.

fn main() {
     let arr = [0; 33];
     
     // this doesn't work because arr's size is bigger than the limit
     // let iv = IVec::from(&arr);

     // this works because it coerces arr into a slice
     let iv = IVec::from(&arr[..]);

     // this doesn't work because it is not implemented for arrays by value
     // let iv = IVec::from(arr);
}

If this change were implemented, all of the above would compile. This also means there will be one less macro, which is usually a good thing. The change is also simple, as both can delegate to the slice implementation (with some potentially more performant options not being breaking changes).

Why not

This would push the MSRV up to 1.51.0, which would be a substantial change. There is also the point that the macro satisfies many uses of the From implementation, and this would be an API change that cannot be reversed without breaking compatibility.

When

Probably not until some other more substantial feature arises that warrants the MSRV bump.

Alternatives

Keep using the macro

Who benefits

Anyone that wants slightly better ergonomics when converting to an IVec (also slightly less messy docs), and a tiny decrease in compile time.

xa888s avatar Mar 11 '21 20:03 xa888s

Rust 1.51.0 was release on March 25, 2021, allowing this change on stable.

Spaceface16518 avatar Mar 27 '21 05:03 Spaceface16518

I would like this change eventually. Historically I've tended to bump the MSRV pretty conservatively after feeling kind of swamped by people telling me things were broken after I cut releases based on new stable versions, and in my mind, something like trailing by 6 months feels appropriate to reduce the number of issues that will be opened by people with old rustc versions.

spacejam avatar Apr 23 '21 15:04 spacejam

A PR for this issue has been made on https://github.com/spacejam/sled/pull/1376. cc @spacejam

mich2000 avatar Oct 04 '21 12:10 mich2000