Add to ArrayVec a from_array_empty Constructor as const fn

[Cryptjar]

This PR adds a new constructor from_array_empty to the ArrayVec, which takes an Array by value and uses it only as storage space, thus initializing with zero length (i.e. empty). Since this is generally valid unlike the from_array_len (which requires an additional check), this check-free version now is eligible to become a const fn to allow ArrayVec even in static context.

However, since const fns may not have any trait bounds (as of Rust 1.51), the A: Array trait bound on the ArrayVec struct itself, is lifted, apparently, it was never used tho.

Since this PR only adds a new function and lifts a constraint, this change should be perfectly backwards compatible.

[Lokathor]

What's happening is that you're lifting the bound on the type but still enforcing it on all the methods of the type. Which does come out to "mostly correct", except that you can no longer assume that the data is A:Array. That is, a person would be allowed to construct an ArrayVec for some data type without it being an array and then all the methods just wouldn't exist for that instantiation.

I'm not sure that I should accept this as is. Removing the bound from the type seems odd to do because now you can construct incorrect ArrayVec values that have no methods.

I could accept this constructor without it being const fn, because it is a useful constructor.

[Cryptjar]

Well, it is not just "mostly correct", it is completely correct. So, if the requirement A:Array is not uphold, as you say, "then all the methods [that require A:Array] just wouldn't exist for that instantiation", because they require it. This is in all sense correct. And since there is no unsafe code in this crate, which might have some additional contracts such as actually assuming that A:Array holds without being able to enforce it with the type system, the type system here really enforces correctness.

Furthermore, well as a rule of thumb, if a struct in the std-lib does not require any trait-bounds it will not have it. Take for example std::io::Cursor, it has no trait-bounds on the struct itself and may be created with any arbitrary T (e.g. Cursor::new(Foo)), yet to be useful it needs at least T: AsRef<[u8]>. And that is enforced where it is actually needed, so that methods like read would just not exist on 'strange' instantiation like a Cursor<Foo>, because it would make no sens, and this way it is correct.

Yet, another example is std::collections::HashMap which also has no trait-bounds on the struct itself, but get & insert will only be available if K: Eq + Hash is upheld on the specific instantiation, otherwise they will simply not be there.

So, from a correctness point of view, there is no need for A:Array on the ArrayVec struct itself, since nobody can ever do anything incorrect with such a struct in Safe Rust. If ever, it might be a point of usability, that a user might unintentionally use an insufficient type (e.g. [u8;42]) with a ArrayVec and only 'later' (that is not by using new_from_empty but instead when using e.g. push) get a compile-time error about trying to use an incorrect type.

On the other hand, by lifting this (functionally irrelevant) constraint, the new_from_empty constructor may become a const fn, which is not usability fanciness, but a true extension of functionality (since without it, nobody can create a const or static containing an ArrayVec). And believe it or not, but that is precisely what I am currently doing (however, based on my fork, yet).

[Lokathor]

However, since it was initially published with the A:Array bound, and since it's marked as repr(C) specifically so that the type's fields can be directly manipulated using unsafe code in other crates, it feels a little off to relax this bound.

I do sympathize with the need to put this in a const or a static though.

Would you be interested in using an alternate approach with having a feature to make the fields of the ArrayVec just be pub? Then you could just make one for the static without anyone needing to change their assumption that A is for sure an array.

[Cryptjar]

However, since it was initially published with the A:Array bound, and since it's marked as repr(C) specifically so that the type's fields can be directly manipulated using unsafe code in other crates, it feels a little off to relax this bound.

I'm not quite sure what you are referring to, since none of the fields are accessible for downstream crates, not even in unsafe. The only thing to do with unsafe downstream is to transmute it to something accessible or pass it to an FFI (which actually may acts as a transmutation). And once it is transmuted (or within an FFI) there also in no requirement for A:Array (or at least it's very unlikely with FFI).

On the other hand the repr(C) does not get influenced by trait-bounds in any way, all that matters stays unchanged by this PR.

But still when using the A:Array bound in downstream crates it still holds that, as the docs say, "that unsafe code must not rely on an instance of this trait being correct", so how much of a contract can any downstream crate imply by this bound? And even if there is a hypothetical downstream unsafe function that requires whatever A:Array can provide (in context of unsafe), when exposed to Safe Rust, this contract should be perfectly expressible via A:Array so if there is a function like:

struct MyVec {/* private magical stuff */}

/// A weird function that needs a `thingy` that upholds a very very important contract.
/// Safety: Only ever call this, if and only if you are sure that **the** contract is uphold
/// (that make this function `unsafe`).
/// However (for whatever reason) this contract is perfectly uphold for any ArrayVec<A> with A:Array
pub unsafe fn weird_foo(thingy: *mut MyVec) {/* do some back magic */}

A perfectly safe wrapper for it would be:

use tinyvec::{Array,ArrayVec};

/// A perfectly safe function (that's the power of Rust)
pub fn a_safe_foo<A:Array>(vec: &mut ArrayVec<A>) {
    unsafe {
        // This is sound, because here, it is enforced that A:Array holds, no matter what,
        // and so the contract is fullfilled
        weird_foo(vec as *mut ArrayVec<A> as *mut MyVec)
    }
}

And so, this wrapper function a_safe_foo may only be called on instance that have a A:Array, and thus always had, no matter what, types don't change magically in Safe Rust. So if anyone manages to call a_safe_foo with via Safe Rust, that vec had been correctly created with some A:Array, independent of the mode of construction that had been used. Notice, that this does not in any way rely on the struct ArrayVec having this bound upfront.

---

Would you be interested in using an alternate approach with having a feature to make the fields of the ArrayVec just be pub? Then you could just make one for the static without anyone needing to change their assumption that A is for sure an array.

I don't think, that this would address your concerns either, because that would really enable anybody to even arbitrarily manipulate all fields of ArrayVec (even if A:Array holds), specifically in Safe Rust.

Imagine that ArrayVec guarantees that self.len <= self.data.as_slice().len() holds (as far as I know this is not an official guarantee, but assume it were). Now, since ArrayVecs fields (currently) are only accessible (via Safe Rust) from within tinyvec a downstream crate might rely on this guarantee (e.g. for FFI). But if now all fields become pub for Safe Rust, now user code, everywhere might alter the ArrayVec::len field to an arbitrary (e.g. too big) value, and suddenly these unsafe functions become unsound (that means incorrect), because the fields can now be arbitrarily altered.

I personally, don't see any good reason for opening these fields, and it might even have real consequences, unlike the approach I propose in this PR.

[rodrimati1992]

The way I've encode marker bounds in const fns on stable is a marker struct that requires the trait to be implemented to be constructed.

So you would get:

// ?Sized because Array doesn't have a Sized supertrait
pub struct IsAnArray<A: ?sized>(PhantomData<fn() -> A>);

impl<A: ?sized + Array> IsAnArray<A> {
     pub const NEW: Self = Self(PhantomData);
}

pub trait Array {
    // Not absolutely required for this workaround
    const IMPLS: IsAnArray<Self> = IsAnArray::NEW;

    ..... rest of the associated items
}

impl<A> ArrayVec<A> {
    /// # Example
    /// 
    /// ```
    /// use tinyvec::{Array, ArrayVec};
    /// 
    /// static ARR: ArrayVec<[u8; 4]> = 
    ///     ArrayVec::from_array_empty([0, 1, 2, 3], Array::IMPLS);
    /// 
    /// assert_eq!(ARR.len(), 0);
    /// assert_eq!(ARR, []);
    /// 
    /// ```
    /// 
    pub const fn from_array_empty(data: A, _: IsAnArray<A>) -> Self {
        Self{data, len: 0}
    }
}

[Lokathor]

Alright I got out voted when I asked around, so we'll go with this.