Hi, I would like to share some changes (improvements) made to nsTArray and the related array classes from the nsTArray.h header file in the last months.
1. Simon added detection for several accidental misuses or disadvantageous uses at compile-time: 1a. https://bugzil.la/1628715: The member functions that return a pointer to an array element either * are now marked with the [[nodiscard]] attribute if they may be nullptr to indicate failure (for the fallible member function overloads), or * are of type mozilla::NotNull<E*> if they cannot be nullptr (for the infallible member function overloads) These changes required adaptations in the existing code at several locations, by either adding missing checks for the overloads that are now [[nodiscard]] or removing redundant nullptr checks for the overloads that now return mozilla::NotNull. New code must take care of this as well, otherwise it will fail to compile. 1b. https://bugzil.la/1628692 and https://bugzil.la/1626570: nsTArray, FallibleTArray and AutoTArray are no longer copy-constructible or copy-assignable. These changes have been made for various reasons: * FallibleTArray was copy-constructible and copy-assignable. However, this was in conflict with the design goal of FallibleTArray to allow only fallible operations. Actually, these operations were implemented fallibly, but they would have failed silently, leaving an empty target array. The alternative of making these operations infallible would have been confusing (However, note that for now, dom::Sequence, which derives from FallibleTArray, remains to be copy-constructible and copy-assignable, but these operations are now infallible, see https://bugzil.la/1631461). * Copying may happen implicitly, but copying an nsTArray may be expensive. Requiring these operations to be explicit increases awareness of these runtime costs. At the same time, there are only relatively few places where copies are actually required. In the course of these changes, several copying code locations have been changed to move, and there remain more opportunities for such improvements. * Correctly implementing copyability when allowing for various types of elements (copyable, copyable and movable, movable) was complex and also non-satisfactory, as doing this prevented the instantiation of nsTArray/... with incomplete element types. The existing code has been adapted to accommodate for the changes. New code must take care of this as well, otherwise it will fail to compile. To summarize the required changes in usage habits: * In some cases, it may actually be possible to move instead of copy. Adding std::move appropriately will be the best solution. * In most cases where moving is not possible, an explicit xvalue copy can be created by calling Clone() on the source array. * Where copy-constructibility/assignability is syntactically required, CopyableTArray resp. CopyableAutoTArray can be used instead. This should only be done where really required, as it opens up again to accidental copies. Some of the following issues were identified as part of these bugs. 2. Simon improved integration with standard C++ algorithms by providing a MakeBackInserter function, which creates an appending output iterator, just like std::back_inserter does for std::vector and the like (https://bugzil.la/1595750 in connection with https://bugzil.la/1623325 and https://bugzil.la/1626206). 3. Emilio removed the need to have a complete definition of the pointee type when calling SafeElementAt in a smart-pointer-typed nsTArray/... in https://bugzil.la/1610066 4. Simon fixed the choice of allocator when calling AppendElements in https://bugzil.la/1630284 5. Simon allowed the fallible variant of EmplaceBack to be actually called in https://bugzil.la/1617604, which required moving the fallible parameter to the beginning of the parameter list 6. Nika relaxed the requirements on mozilla:Span parameters passed to various nsTArray/... methods to allow them being called with Span with non-const element types in https://bugzil.la/1630777 7. Simon made it possible to move a nsTArray into a FallibleTArray with a move-only element type in https://bugzil.la/1620632, which previously failed to compile on older gcc/libstdc++ versions such as the base-toolchain. Some of the changes necessary to support this have subsequently been reverted by the above-mentioned changes related to copyability of nsTArray/... 8. The types and macros related to memtools performance optimizations were renamed to better reflect their current effect as part of https://bugzil.la/1620632: * DECLARE_USE_COPY_CONSTRUCTORS is now MOZ_DECLARE_RELOCATE_USING_MOVE_CONSTRUCTOR * nsTArray_CopyChooser is now nsTArray_RelocationStrategy * nsTArray_CopyWithConstructors is now nsTArray_RelocateUsingMoveConstructor Some additional internal improvements have been made as well (e.g. André replaced several Mozilla-specific type-traits by those from <type_traits> in https://bugzil.la/1626200 and https://bugzil.la/1625138, Chris replaced MOZ_MUST_USE by the [[nodiscard]] attribute in Bug https://bugzil.la/1624776, and Emilio replaced mozilla::Swap by std::swap in https://bugzil.la/1609996). If you have any questions/suggestions about these changes, please share them or file a bug. Best wishes Simon _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
