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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/ext/__hash:13
 
 #  pragma GCC system_header
 
----------------
I notice this is not guarded correctly by `_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER`. We should fix that, but probably fix it //after// the dust has settled on all these reviews.


================
Comment at: libcxx/utils/libcxx/test/params.py:80
               AddCompileFlag('-fcxx-modules'), # AppleClang disregards -fmodules entirely when compiling C++. This enables modules for C++.
+              AddCompileFlag('-D_LIBCXX_MODULES_BUILD') # Needed to exclude headers that are not supported in a modules build
             ] if modules else []),
----------------
I think this macro should have the word `TEST` in it somewhere.

However, since you use it only for the generated header tests, IMHO it would be quite reasonable to just rip out the ext/ headers from those generated tests specifically (i.e. adopt a small portion of D120831's changes), and then you don't need the macro at all, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118616