[libcxx] span: Cleanup includes

[miscco]

Summary

Remove the duplicated include of <cstddef> and forward declare std::array rather than dragging in the header.

[mclow.lists]

I'm fine with the other changes here, but not the array change.

libcxx/include/span

148

Forward declaring array will lead to other problems down the road.
Visibility problems. Which lead to ODR violations.

[mclow.lists]

libcxx/include/span

132

Let's make the comment "for ptrdiff_t and byte"

[miscco]

Some comments

libcxx/include/span

132

As far as I can see ptrdiff_t was only needed for the old index_type. So I would ommit it here.

148

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.

[mclow.lists]

libcxx/include/span

148

if you look in <array>, you will see the following declaration:

template <class _Tp, size_t _Size>
struct _LIBCPP_TEMPLATE_VIS array

Your forward declaration is different.

Also, for some definitions of _LIBCPP_TEMPLATE_VIS, it can only be on the first declaration of the template in any translation unit. How would you enforce this?

[bcraig]

libcxx/include/span

148

A solution to this would be to have an <array_fwd> header (or perhaps even a general <libcxx_fwd> header) that had all the expensive forward declarations, then ensured that <array> pulled in <__array_fwd>.

Difficulty: This increases the number of headers that clients pull in. I don't personally think that's a big deal, but I seem to recall that the quantity of headers is something that this project tends to be picky about.

[miscco]

Do not forward declare array

[miscco]

Removed the forward declaration of array as that would be far more involved.

[ldionne]

Looks like you implemented the requested changes, and this LGTM. Thanks!

FWIW, I'm favorable to the idea of having forward declaration headers, however I think we have a lot of work to do before this makes sense, like splitting up the header files containing definitions. I think I would like to see this done as a concentrated effort instead of ad-hoc patches.

libcxx/include/span

 template<class Container>
     span(const Container&) -> span<const typename Container::value_type>;
 
 } // namespace std
 
 */
 
 #include <__config>
-#include <cstddef>      // for ptrdiff_t
+#include <cstddef>      // for byte
 #include <iterator>     // for iterators
-#include <array>        // for array
 #include <type_traits>  // for remove_cv, etc
-#include <cstddef>      // for byte
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 17
 
 inline constexpr size_t dynamic_extent = numeric_limits<size_t>::max();
 template <typename _Tp, size_t _Extent = dynamic_extent> class span;
 
+template <class _Tp, size_t _Sz>
+struct array;
 
 template <class _Tp>
 struct __is_span_impl : public false_type {};
 
 template <class _Tp, size_t _Extent>
 struct __is_span_impl<span<_Tp, _Extent>> : public true_type {};
 
 template <class _Tp>

libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp

 //
 // Remarks: These constructors shall not participate in overload resolution unless:
 //   — extent == dynamic_extent || N == extent is true, and
 //   — remove_pointer_t<decltype(data(arr))>(*)[] is convertible to ElementType(*)[].
 //
 
 
 #include <span>
+#include <array>
 #include <cassert>
 #include <string>
 
 #include "test_macros.h"
 // std::array is explicitly allowed to be initialized with A a = { init-list };.
 // Disable the missing braces warning for this reason.
 #include "disable_missing_braces_warning.h"