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

Tue Mar 1 09:46:40 PST 2022

EricWF 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
----------------
philnik wrote:
> 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.
But there's a `#pragma system_header`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118616