[PATCH] D119439: [libc++] Remove __functional_base

Nikolas Klauser via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 10 14:00:30 PST 2022


philnik added inline comments.


================
Comment at: libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp:20
+#include <cstdint>
+#include <ranges>
 
----------------
ldionne wrote:
> philnik wrote:
> > Quuxplusone wrote:
> > > Please leave `<ranges>` first; historically this is our style for tests that test a particular header (although I admit I don't see much point to it).
> > I didn't know that rule and nobody has said anything in my other PRs until now, so I'd like to know @ldionne's opinion on this. I don't see much point to it either.
> That's true, we've always done this (rather blindly TBH). We may not have noticed on your other PRs.
> 
> The benefit of having the header first is that we make sure the header can be included when nothing else has been included before. I'm fine with not doing this anymore in our tests, however I think it would be nice to also have generated tests that simply include every single public header in an otherwise empty translation unit, just to make sure it works.
> 
> TLDR: Let's test this properly and then we can safely drop this weird custom we have. I'm fine with this patch not following the usual rule.
You mean something like `test/libcxx/inclusions`? (and extend it to generate tests for all headers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119439



More information about the llvm-commits mailing list