Make FixedSizeArray an unsafe trait

[alevy]

[breaking-change]

FixedSizeArray is meant to be implemented for arrays of fixed size only, but can be implemented for anything at the moment. Marking the trait unsafe would make it more reasonable to write unsafe code which operates on fixed size arrays of any size.

For example, using uninitialized to create a fixed size array and immediately filling it with a fixed value is externally safe:

pub fn init_with_nones<T, A: FixedSizeArray<Option<T>>>() -> A {
    let mut res = unsafe { mem::uninitialized() };
    for elm in res.as_mut_slice().iter_mut() {
        *elm = None;
    }
    res
}

But the same code is not safe if FixedSizeArray is implemented for other types:

struct Foo { foo: usize }
impl FixedSizeArray<Option<usize>> for Foo {
    fn as_slice(&self) -> &[usize] { &[] }
    fn as_mut_slice(&self) -> &mut [usize] { &mut [] }
}

now init_with_nones() : Foo returns a Foo with an undefined value for the field foo.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[bluss]

Can you put [breaking-change] (exactly) in the commit log or the PR message? Thanks.

This sounds good to me, but I don't think I can make the decision.

[alevy]

@bluss I added it to the PR message, hopefully correctly...

[petrochenkov]

Hm, if you break it anyway maybe you can make T an output parameter? FixedSizeArray<T> and FixedSizeArray<U> where T != U can never be implemented for the same type.

[alevy]

@petrochenkov Seems reasonable except I'm not able to write this such that it comiles atm:

unsafe impl<E, A: Unsize<[E]>> FixedSizeArray for A {
    type T = A;

    #[inline]
    fn as_slice(&self) -> &[Self::T] {
        self
    }
    #[inline]
    fn as_mut_slice(&mut self) -> &mut [Self::T] {
        self
    }
}

Results in a compile error citing E0207 -- that E is not bound by the impl trait, self type or perdicate. This may be a bug in RFC 447, which doesn't seem to consider bounds by other types (in this case, the type of E is determined uniquely, I think)

[bluss]

@alexcrichton I'd like us to do this

[petrochenkov]

@alevy
That's bad. There may be a workaround, but I don't know it (people on IRC may know). The patch looks good enough to me even without this change.

[alexcrichton]

Can you also expand the documentation to indicate why this trait is unsafe? e.g what guarantees must implementors uphold and what guarantees can code rely on?

I'm a little wary to continue to tweak perma-unstable traits like this because they're permanently unstable and lots of tweaks mean that the abstraction is useful and should perhaps live externally on crates.io first before moving into the standard library. This trait was added some time ago and there's no current usage in-tree, so there's no real reason this can't be external.

For now though an incremental improvement is fine.

[alevy]

@alexcrichton I expanded the documentation. Happy to revise if needed, though. Thanks!

[alexcrichton]

@bors: r+ b30d896

Thanks!

[bors]

⌛ Testing commit b30d896 with merge 6a21874...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt