[libcxx-commits] [PATCH] D105597: [libcxx][nfc] Remove <variant>'s dependence on <array>.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/variant:242
+struct __light_array {
+  _Tp __buff_[_Size] = {0};
+
----------------
What is `_Tp` here? Are we sure `0` is convertible to `_Tp`? (Is `_Tp` always just `size_t`? That'd be super convenient. :))
Are we sure `_Size` is never zero?


================
Comment at: libcxx/include/variant:244
+
+  inline _LIBCPP_INLINE_VISIBILITY constexpr
+  _Tp &operator[](size_t __n) noexcept {
----------------
Nit: here and line 249, `inline` is not needed because these methods are implicitly inline.


================
Comment at: libcxx/include/variant:254-261
+  inline _LIBCPP_INLINE_VISIBILITY
+  constexpr friend bool
+  operator==(const __light_array& __x, const __light_array& __y) noexcept {
+      for (size_t __i = 0; __i < _Size; ++__i)
+        if (__x.__buff_[__i] != __y.__buff_[__i])
+          return false;
+      return true;
----------------
I wonder why we need `==` to work on this type.


================
Comment at: libcxx/include/variant:541
+    using __result = __light_array<common_type_t<__uncvref_t<_Fs>...>, sizeof...(_Fs)>;
+    return __result{_VSTD::forward<_Fs>(__fs)...};
   }
----------------
I see no reason to drop the double-`{{` here. I think it works either way, but the single-`{` version is using magic brace-inferring logic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105597/new/

https://reviews.llvm.org/D105597