[clangd] Log all ignored diagnostics.

[ilya-biryukov]

Summary

To aid debugging failures and crashes.
Only part of ignored diagnostics was logged before, now we log all of
them.

[hokein]

clangd/ClangdUnit.cpp

197 ↗

(On Diff #133591)

I'm not sure, do we care about this particular case (diagnostic outside main file)? With the new change, we don't know whether the diag is from main file or outside main file。

clangd/Compiler.h

27 ↗

(On Diff #133591)

maybe just name it log? since this method is already in IgnoreDiagnostics class.

[ilya-biryukov]

• Renamed logIgnoredDiag to log.

clangd/ClangdUnit.cpp

197 ↗

(On Diff #133591)

We probably don't.
We output the location of the diagnostic anyway, so there should always be enough context when looking at the logs.

[hokein]

LGTM.

clangd/ClangdUnit.cpp

197 ↗

(On Diff #133591)

Ah, make sense.

clang-tools-extra/trunk/clangd/ClangdUnit.cpp

 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
-#include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 using namespace clang::clangd;
 using namespace clang;
 
 namespace {
 
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
   Result.range = toRange(Lexer::makeFileCharRange(FixIt.RemoveRange, M, L), M);
   Result.newText = FixIt.CodeToInsert;
   return Result;
 }
 
 llvm::Optional<DiagWithFixIts> toClangdDiag(const clang::Diagnostic &D,
                                             DiagnosticsEngine::Level Level,
                                             const LangOptions &LangOpts) {
-  SmallString<64> Message;
-  D.FormatDiagnostic(Message);
-
   if (!D.hasSourceManager() || !D.getLocation().isValid() ||
       !D.getSourceManager().isInMainFile(D.getLocation())) {
-
-    SmallString<64> Location;
-    if (D.hasSourceManager() && D.getLocation().isValid()) {
-      auto &SourceMgr = D.getSourceManager();
-      auto Loc = SourceMgr.getFileLoc(D.getLocation());
-      llvm::raw_svector_ostream OS(Location);
-      Loc.print(OS, SourceMgr);
-    } else {
-      Location = "<no-loc>";
-    }
-
-    log(llvm::formatv("Ignored diagnostic outside main file. {0}: {1}",
-                      Location, Message));
+    IgnoreDiagnostics::log(Level, D);
     return llvm::None;
   }
 
+  SmallString<64> Message;
+  D.FormatDiagnostic(Message);
+
   DiagWithFixIts Result;
   Result.Diag.range = diagnosticRange(D, LangOpts);
   Result.Diag.severity = getSeverity(Level);
   Result.Diag.message = Message.str();
   for (const FixItHint &Fix : D.getFixItHints())
     Result.FixIts.push_back(toTextEdit(Fix, D.getSourceManager(), LangOpts));
   return std::move(Result);
 }

clang-tools-extra/trunk/clangd/Compiler.h

 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 
 namespace clang {
 namespace clangd {
 
 class IgnoreDiagnostics : public DiagnosticConsumer {
 public:
+  static void log(DiagnosticsEngine::Level DiagLevel,
+                  const clang::Diagnostic &Info);
+
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                        const clang::Diagnostic &Info) override {}
+                        const clang::Diagnostic &Info) override;
 };
 
 /// Creates a compiler instance, configured so that:
 ///   - Contents of the parsed file are remapped to \p MainFile.
 ///   - Preamble is overriden to use PCH passed to this function. It means the
 ///     changes to the preamble headers or files included in the preamble are
 ///     not visible to this compiler instance.
 ///   - vfs::FileSystem is used for all underlying file accesses. The actual

clang-tools-extra/trunk/clangd/Compiler.cpp

 //===--- Compiler.cpp -------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include "Compiler.h"
+#include "Logger.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
 
+void IgnoreDiagnostics::log(DiagnosticsEngine::Level DiagLevel,
+                            const clang::Diagnostic &Info) {
+  SmallString<64> Message;
+  Info.FormatDiagnostic(Message);
+
+  SmallString<64> Location;
+  if (Info.hasSourceManager() && Info.getLocation().isValid()) {
+    auto &SourceMgr = Info.getSourceManager();
+    auto Loc = SourceMgr.getFileLoc(Info.getLocation());
+    llvm::raw_svector_ostream OS(Location);
+    Loc.print(OS, SourceMgr);
+    OS << ":";
+  }
+
+  clangd::log(llvm::formatv("Ignored diagnostic. {0}{1}", Location, Message));
+}
+
+void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                                         const clang::Diagnostic &Info) {
+  IgnoreDiagnostics::log(DiagLevel, Info);
+}
+
 std::unique_ptr<CompilerInstance>
 prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
                         const PrecompiledPreamble *Preamble,
                         std::unique_ptr<llvm::MemoryBuffer> Buffer,
                         std::shared_ptr<PCHContainerOperations> PCHs,
                         IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                         DiagnosticConsumer &DiagsClient) {
   assert(VFS && "VFS is null");