Impl Reflect for Box<[T]>

[tguichaoua]

Objective

• Fixes Implement Reflect for Box<[T]> #11570.

Solution

Implements one the solutions proposed in #11570 (comment).

• Add FixedLenList as super trait of List.

---

Changelog

• Added FixedLenList trait as a super trait of List which contains all the methods that did not change the size of the list (e.g. insert, remove, etc.).
• Implement Reflect for Box<[T]> as a FixedLenList.

Migration Guide

• ReflectRef, ReflectMut and ReflectOwned have a new variant named FixedLenList.
  Accessing a fixed len list from one of these enum may require some boilerplate code :

let list_ref = match b.reflect_ref() {
    ReflectRef::FixedLenList(list) => list,
    ReflectRef::List(list) => list.as_fixed_len_list(),
    _ => unimplemented!(),
};

• Every types that implement List must also implement FixedLenList
• The following methods have been migrated from List to FixedLenList
  • get
  • get_mut
  • len
  • is_empty
  • iter
  • clone_dynamic

[doonv]

I like this, but I'm not particularly a fan of the FixedLenList name. But I don't have a better name to suggest.

[tguichaoua]

I like this, but I'm not particularly a fan of the FixedLenList name. But I don't have a better name to suggest.

Yes me too.
Another idea I had is to name the base trait List and the resizable list one something like ResizableList.
But it's probably confusing to rename an existing trait and introduce a new trait with the old name.

[Adamkob12]

name suggestions / general ideas:

• List -> DyanmicList , FixedLenList -> List
• "Collection"
• "SizedList"
• "DynamicArray"
• "Slice"

but I also think that FixedSizeList / FixedLenList are clear enough - I wouldn't mind them.

[pablo-lua]

Liked this changes and this sure can be useful in importing the std types into reflection, but there is something bugging me, and commented on that.

[pablo-lua]

name suggestions / general ideas:

[...]

• "SizedList"

Liked the name SizedList, but agree that FixedLenList is a good name too. Another possible name is FixedSizeList if we can't follow with Len for some reason

[pablo-lua]

I'm on board with this, the user can probably do some performances improvements with something that they knows that has a fixed len. And helping the user to get reflection on Box<[T]> is good too.

[spectria-limina]

Why not just Array? In Rust parlance, an array has a static size, so it seems like a natural complement to List being the dynamic one.

[doonv]

Why not just Array? In Rust parlance, an array has a static size, so it seems like a natural complement to List being the dynamic one.

Because an Array means the size is known at compile time, a Box<[T]> is a fixed length list with a size known at runtime instead of compile time.

[spectria-limina]

Ahh... SizedList, perhaps? Though that could have the same problem...

[soqb]

gah! another Cow<Foo>/Box<Foo>/&Foo inconsistency. starting to think we should somehow have ReflectUnsized/ReflectBorrow traits which make things easier.

regarding the implementation, i would rather we used List and bit the performance pain point of just reallocating via Vec (using this From impl and Vec::into_boxed_slice) on every insertion since i think trait complexity is already extremely high in Reflect (and only going up).

[spectria-limina]

It may be a good idea to start with a simple implementation and revisit later if it's proving a hotspot

[SpecificProtagonist]

Because an Array means the size is known at compile time, a Box<[T]> is a fixed length list with a size known at runtime instead of compile time.

Reflection can only be accessed at runtime, so Array and the proposed FixedLenList both are the same.

[NthTensor]

This PR appears to have gone stale. Nominating to close.

[SOF3]

I would like to rework on this if necessary. What is blocking on this PR (other than author inactive)? Was the naming bikeshed unresolved?