[libcxx-commits] [PATCH] D72036: [libcxx] span: Cleanup includes

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco marked 2 inline comments as done.
miscco added a comment.

Some comments



================
Comment at: libcxx/include/span:132
 #include <__config>
-#include <cstddef>      // for ptrdiff_t
+#include <cstddef>      // for byte
 #include <iterator>     // for iterators
----------------
mclow.lists wrote:
> Let's make the comment "for ptrdiff_t and byte"
As  far as I can see  ptrdiff_t was only needed for the old index_type. So I would ommit it here.


================
Comment at: libcxx/include/span:148
+template <class _Tp, size_t _Sz>
+struct array;
 
----------------
mclow.lists wrote:
> Forward declaring `array` will lead to other problems down the road. 
> Visibility problems. Which lead to ODR violations.
> 
I would disagree. It is essentially only used by the deduction guides. So to use it one has to already have array included.

I would say the other way is surprising as shown in the test, where the array header was not included but used.

That said I would not really make a stand Herr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72036