ldionne added a comment. In D91311#2423809 <https://reviews.llvm.org/D91311#2423809>, @rsmith wrote:
> In D91311#2418881 <https://reviews.llvm.org/D91311#2418881>, @ldionne wrote: > >> In D91311#2417293 <https://reviews.llvm.org/D91311#2417293>, @rsmith wrote: >> >>> In D91311#2416940 <https://reviews.llvm.org/D91311#2416940>, @ldionne wrote: >>> >>>> LGTM from the libc++ point of view. The CI is passing -- those failures >>>> are flaky modules tests that we need to fix. >>> >>> Perhaps we need to specify `-fmodules-validate-system-headers` in the test >>> so Clang doesn't assume that system headers are unchanged? >> >> Oh, would that be it? We're not including libc++ headers as system headers >> when running the tests, though (and we're disabling the system header >> pragma). Do you think that might still be the issue? > > The module map lists `std` as being `[system]` module, so I think the headers > will still end up being treated as system headers. So yes, I think there's a > possibility that is still the issue. > >> I tried a dumb workaround with D92131 <https://reviews.llvm.org/D92131>, but >> it's arguably not great. I'd love to have your thoughts on that. > > I'm not sure that'll make any difference: at least in my setup, `%t` expands > to the same path on subsequent invocations of the same test, so you'll still > reuse module caches from one test run to the next. In Clang tests for this > sort of thing, we have explicit `RUN: rm -rf %t/ModuleCache` lines to clean > up any stale module cache before running a test... but that shouldn't really > be necessary; Clang should be able to detect when the files are out of date. Ok, thanks for the info. I'll use `-fmodules-validate-system-headers` instead. Anyway, this patch LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91311/new/ https://reviews.llvm.org/D91311 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
