[libcxx-commits] [PATCH] D102135: [libcxx][ranges] adds _`copyable-box`_

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/libcxx/ranges/range.adaptors/range.copy.wrap/types.h:13
+#include <cassert>
+#include <__ranges/copyable_box.h>
+
----------------
ldionne wrote:
> Quuxplusone wrote:
> > I'd rather see just `#include <ranges>` here.
> Can you explain why? Does that apply to the other test files too?
> 
> I like including `<__ranges/copyable_box.h>` since it's an implementation-detail type, and it has the added benefit that it tests that the header can be included on its own.
(A) I guess I'm neutral now. If you do `<ranges>`, then the test is more future-proof against future renamings of internal headers; but of course it's deliberately not future-proof against future renamings of internal //types//, so maybe who cares. I do believe we both agree that we don't want to see arbitrary test cases including detail headers, though, right? If we keep the detail header here, we're saying it's a special case because we're specifically testing `__copyable_box`, not setting a precedent for including detail headers in test code.

(B) "tests that the header can be included on its own" — I recently had a shower thought that I ought to write a little Python script to list all the detail headers and for each of them, try compiling a one-line TU including just that header, to help ensure IWYU hygiene. I might even do that this weekend. But I don't think there's any way to bolt such a script onto our actual test suite; it'd just be a one-off local experiment. (Better would probably be to run the codebase through one of those IWYU-enforcing analysis tools, right? I have no experience with such tools.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135