[PATCH] D106124: [libcxx][modules] protects users from relying on detail headers

[Louis Dionne via Phabricator via llvm-commits]

ldionne added a comment.

Nevermind, the above was wrong too! It turns out to be much trickier than this. In fact, we pass `-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER` when running the tests with Clang, which means that we're excluding all the `include_instead` pragmas. So this is basically never tested, except in the generated tests where we explicitly undefine `_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER`.

Oh, and it's also "tested" on AppleClang because we don't define `_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER` with that compiler. I suspect that's why these issues showed up locally with a recent AppleClang but not on the Linux CI (nor on the Apple CI, which might be using an older AppleClang that doesn't support the pragma).

So... long story short, I think what we want is:

1. Pass `_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER` with AppleClang (easy to do and I'll handle that separately).
2. Use `-I` instead of `-isystem` in the test suite (D118616 <https://reviews.llvm.org/D118616> does that).
3. Use `pragma include_instead` regardless of whether `_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER` is defined (I don't know why it's guarded by it anyways).

Until we get ourselves out of this mess a bit more, I think the only option is to revert this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106124