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

Mon Feb 28 06:02:39 PST 2022

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Thanks for commandeering. Can you also change the testing configurations in `libcxxabi/test/configs/`?

================
Comment at: libcxx/include/__config:94-96
+// These macros are defined in some tests
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wmacro-redefined")
----------------
Quuxplusone wrote:
> Then personally I'd prefer to pass `-Wno-macro-redefined` in those specific tests. We shouldn't do anything in `<__config>` that is beneficial only for certain tests in the test suite; we should do it here iff we think it'll benefit real users. Do we want real users to be able to do, like, `-D_LIBCPP_ABI_ALTERNATE_STRING_LAYOUT=1`?  If so, then lines 95-96 are good but the comment on line 94 should talk about //that//, not about tests in the test suite.
+1 here, we should just add those suppressions in the tests that need them

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118616