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

[Ben Craig via Phabricator via libcxx-commits]

bcraig added inline comments.


================
Comment at: libcxx/include/span:148
+template <class _Tp, size_t _Sz>
+struct array;
 
----------------
mclow.lists wrote:
> miscco wrote:
> > 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.
> 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?
> 
> 
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72036