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

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 284811.
adamcz added a comment.

log dropped dianostic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85753

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/ModulesTests.cpp


Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -39,6 +39,34 @@
   TU.index();
 }
 
+TEST(Modules, Diagnostic) {
+  // 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(
+    /*error-ok*/
+    #include "modular.h"
+
+    void bar() {}
+)cpp");
+  TU.OverlayRealFileSystemForModules = true;
+  TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap"));
+  TU.ExtraArgs.push_back("-fmodules");
+  TU.ExtraArgs.push_back("-fimplicit-modules");
+  TU.ExtraArgs.push_back("-fmodules-strict-decluse");
+  TU.AdditionalFiles["modular.h"] = R"cpp(
+    #include "non-modular.h"
+  )cpp";
+  TU.AdditionalFiles["non-modular.h"] = "";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+    module M {
+      header "modular.h"
+    }
+)modulemap";
+
+  // Test that we do not crash.
+  TU.build();
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -122,7 +122,8 @@
   // The ClangTidyContext populates Source and Name for clang-tidy diagnostics.
   std::vector<Diag> take(const clang::tidy::ClangTidyContext *Tidy = nullptr);
 
-  void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override;
+  void BeginSourceFile(const LangOptions &Opts,
+                       const Preprocessor *PP) override;
   void EndSourceFile() override;
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const clang::Diagnostic &Info) override;
@@ -148,6 +149,7 @@
   llvm::Optional<Diag> LastDiag;
   llvm::Optional<FullSourceLoc> LastDiagLoc; // Valid only when LastDiag is set.
   bool LastDiagOriginallyError = false;      // Valid only when LastDiag is set.
+  SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
   bool LastPrimaryDiagnosticWasSuppressed = false;
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -518,13 +518,17 @@
 }
 
 void StoreDiags::BeginSourceFile(const LangOptions &Opts,
-                                 const Preprocessor *) {
+                                 const Preprocessor *PP) {
   LangOpts = Opts;
+  if (PP) {
+    OrigSrcMgr = &PP->getSourceManager();
+  }
 }
 
 void StoreDiags::EndSourceFile() {
   flushLastDiag();
   LangOpts = None;
+  OrigSrcMgr = nullptr;
 }
 
 /// Sanitizes a piece for presenting it in a synthesized fix message. Ensures
@@ -560,6 +564,14 @@
 
 void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                                   const clang::Diagnostic &Info) {
+  // If the diagnostic was generated for a different SourceManager, skip it.
+  // This can happen when using implicit modules.
+  if (OrigSrcMgr && Info.hasSourceManager() &&
+      OrigSrcMgr != &Info.getSourceManager()) {
+    IgnoreDiagnostics::log(DiagLevel, Info);
+    return;
+  }
+
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
   bool OriginallyError =
       Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85753.284811.patch
Type: text/x-patch
Size: 3589 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200811/7c016633/attachment.bin>