[PATCH] D85753: [clangd] Discard diagnostics from another SourceManager.

[Sam McCall via Phabricator via cfe-commits]

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