[libcxx-commits] [PATCH] D118616: [libc++] Use -I instead of -isystem to include headers in the test suite

Mon Feb 28 15:22:28 PST 2022

philnik added inline comments.

================
Comment at: libcxx/include/ext/__hash:19-22
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#include <__string>
+_LIBCPP_DIAGNOSTIC_POP
----------------
ldionne wrote:
> philnik wrote:
> > Quuxplusone wrote:
> > > Here and below, this shouldn't be necessary. Remember the economist's $100 bill: if this were needed, why wouldn't we be doing it everywhere? (E.g., in `<string>`.) It seems likely that there's some master list of "libc++ headers" somewhere and that `<ext/__hash>` and `<ext/hash_map>` are accidentally missing from that list.
> > I thought the `<ext/*>` headers weren't in `module.modulemap`, because they were deprecated. It's also possible they are not in there by accident.
> I don't understand why this diff is necessary in this patch. What happens if you remove it? What fails?
Because we replace `-isystem` with `-I`. All warnings in headers are now part of the tests, so there must not be any warnings because of `-Werror`. So basically, the modules build would fail.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118616