teslamotors / fixed-containers

C++ Fixed Containers
MIT License
395 stars 38 forks source link

EnumArray Ranges Constructor Should Forward Values #82

Closed younesr1 closed 1 year ago

younesr1 commented 1 year ago

Added test for enumarray range constructor with moveonly types. Confirmed that the test fails to compile without the newly added std::forwarded since it tries to make a copy of the moveonly type.

alexkaratarakis commented 1 year ago

Judging by std::vector's equivalent constructors, it seems like the constructor should not attempt to move by default.

Reasons: 1) The spec and cppreference don't explicitly clarify about the constructor, but the _range functions that are added along with the new constructors do explicitly state that it is supposed to be copies, for example link

2) msvc has already implemented range-based construction in std::vector - it doesn't do moving: https://godbolt.org/z/MP83W73f3

3) Iterator-based moving has been opt-in and explicit, with std::move_iterator.

younesr1 commented 1 year ago

Thanks for the help Alex! I'll close this PR since it's really important we keep parity with how the STL does things. (I am still salty the STL does it this way though lol).