sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
Yeah I think this is the right approach for now, because the cached modules don't include diagnostics and ensuring that they're persisted and available is a larger project. ================ Comment at: clang-tools-extra/clangd/Diagnostics.cpp:568 + // If the diagnostic was generated for a different SourceManager, skip it. + // This can happen when using implicit modules. + if (OrigSrcMgr && Info.hasSourceManager() && ---------------- maybe a FIXME: errors from implicitly built modules should be surfaced somehow (but then must also be surfaced when the module was cached) ================ Comment at: clang-tools-extra/clangd/Diagnostics.cpp:568 + // If the diagnostic was generated for a different SourceManager, skip it. + // This can happen when using implicit modules. + if (OrigSrcMgr && Info.hasSourceManager() && ---------------- sammccall wrote: > maybe a FIXME: errors from implicitly built modules should be surfaced > somehow (but then must also be surfaced when the module was cached) Can we be a bit more specific: this happens when an #include causes a module to be implicitly built, using a separate SourceManager. ================ Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp:44 + // Produce a diagnostic while building an implicit module. Use + // -fmodules-strick-decluse, but any non-silenced diagnostic will do. + TestTU TU = TestTU::withCode(R"cpp( ---------------- nit: strict Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85753/new/ https://reviews.llvm.org/D85753 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
