[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

149

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

[mclow.lists]

libcxx/include/span

131–132

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

[miscco]

Some comments

libcxx/include/span

131–132

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

149

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

149

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

149

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 <iterator>     // for iterators
 #include <array>        // for array
-#include <type_traits>  // for remove_cv, etc
 #include <cstddef>      // for byte
+#include <iterator>     // for iterators
+#include <type_traits>  // for remove_cv, etc
 
 #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>
 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>
 struct __is_span : public __is_span_impl<remove_cv_t<_Tp>> {};