[clangd] Dont work on diags if we are not going to emit

[kadircet]

Summary

Doesn't install clang-tidy checks & process clang diags when they're
going to be dropped. Also disables analysis for some warnings completely.

[kadircet]

• Disable IncludeFixer too.

[sammccall]

Great!

clang-tools-extra/clangd/ParsedAST.cpp

300

These explicit upcasts seem ugly enough that I might extract a pointer var to avoid them...

342–345

s/initialize a clang-tidy context/run clang-tidy/

clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

464

Why are these tests deleted?

[kadircet]

clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

464

they rely on the fact that clang-tidy checkers PPcallbacks are run with patched asts, but it is no longer the case, hence it becomes impossible to satisfy them.

currently we never replay includes with patched preambles, as ReplayPreamble bails out when there are no existing PPCallbacks and that's always the case. we can still try and test it via complicated means like enabling ClangdFeatures to register PPCallbacks, but it will be testing a feature that doesn't exist in practice + would be quite some work for just testing, so I'd rather leave it as-is (probably by adding a comment around ReplayPreamble::attach saying that we should test this once there are non-clang-tidy users).

[sammccall]

clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

464

Oops, I didn't scroll up fast enough (reviewing on phone lol). SGTM

[kadircet]

• Get rid of static_casts
• Update comments

[kadircet]

• Revert revert of IncludeFixer changes

clang-tools-extra/clangd/ParsedAST.cpp

ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
   ASTDiags.setDiagCallback(
       [&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
         llvm::for_each(ASTListeners,
                        [&](const auto &L) { L->sawDiagnostic(D, Diag); });
       });
 
   llvm::Optional<PreamblePatch> Patch;
   bool PreserveDiags = true;
+  // We might use an ignoring diagnostic consumer if they are going to be
+  // dropped later on to not pay for extra latency by processing them.
+  DiagnosticConsumer *DiagConsumer = &ASTDiags;
+  IgnoreDiagnostics DropDiags;
   if (Preamble) {
     Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
     Patch->apply(*CI);
     PreserveDiags = Patch->preserveDiagnostics();
+    if (!PreserveDiags)
+      DiagConsumer = &DropDiags;
   }
   auto Clang = prepareCompilerInstance(
       std::move(CI), PreamblePCH,
       llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
-      ASTDiags);
+      *DiagConsumer);
   if (!Clang) {
     // The last diagnostic contains information about the reason of this
     // failure.
     std::vector<Diag> Diags(ASTDiags.take());
     elog("Failed to prepare a compiler instance: {0}",
          !Diags.empty() ? static_cast<DiagBase &>(Diags.back()).Message
                         : "unknown error");
     return None;
   }
+  if (!PreserveDiags) {
+    // Skips some analysis.
+    Clang->getDiagnosticOpts().IgnoreWarnings = true;
+  }
 
   auto Action = std::make_unique<ClangdFrontendAction>();
   const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
   if (!Action->BeginSourceFile(*Clang, MainInput)) {
     log("BeginSourceFile() failed when building AST for {0}",
         MainInput.getFile());
     return None;
   }
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
   // Clang-tidy has some limitations to ensure reasonable performance:
   //  - checks don't see all preprocessor events in the preamble
   //  - matchers run only over the main-file top-level decls (and can't see
   //    ancestors outside this scope).
   // In practice almost all checks work well without modifications.
   std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
   ast_matchers::MatchFinder CTFinder;
   llvm::Optional<tidy::ClangTidyContext> CTContext;
-  {
+  llvm::Optional<IncludeFixer> FixIncludes;
+  // No need to run clang-tidy or IncludeFixerif we are not going to surface
+  // diagnostics.
+  if (PreserveDiags) {
     trace::Span Tracer("ClangTidyInit");
     tidy::ClangTidyOptions ClangTidyOpts =
         getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
     dlog("ClangTidy configuration for file {0}: {1}", Filename,
          tidy::configurationAsText(ClangTidyOpts));
     tidy::ClangTidyCheckFactories CTFactories;
     for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
       E.instantiate()->addCheckFactories(CTFactories);
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
           if (DiagLevel == DiagnosticsEngine::Warning &&
               CTContext->treatAsError(CheckName)) {
             return DiagnosticsEngine::Error;
           }
         }
       }
       return DiagLevel;
     });
-  }
 
     // Add IncludeFixer which can recover diagnostics caused by missing includes
     // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
-  llvm::Optional<IncludeFixer> FixIncludes;
     auto BuildDir = VFS->getCurrentWorkingDirectory();
     if (Inputs.Index && !BuildDir.getError()) {
-    auto Style = getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
+      auto Style =
+          getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
       auto Inserter = std::make_shared<IncludeInserter>(
           Filename, Inputs.Contents, Style, BuildDir.get(),
           &Clang->getPreprocessor().getHeaderSearchInfo());
       if (Preamble) {
         for (const auto &Inc : Preamble->Includes.MainFileIncludes)
           Inserter->addExisting(Inc);
       }
       FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
                           /*IndexRequestLimit=*/5);
       ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
                                               const clang::Diagnostic &Info) {
         return FixIncludes->fix(DiagLevl, Info);
       });
       Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
     }
+  }
 
   IncludeStructure Includes;
   // If we are using a preamble, copy existing includes.
   if (Preamble) {
     Includes = Preamble->Includes;
     Includes.MainFileIncludes = Patch->preambleIncludes();
     // Replay the preamble includes so that clang-tidy checks can see them.
     ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,

clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

EXPECT_EQ(SM.getDecomposedLoc(SkippedFiles[I].location()).second,
               Inc.FileNameOffset);
     // This also contains quotes/angles so increment the range by one from both
     // sides.
     EXPECT_EQ(
         SkippedFiles[I].text(SM),
         Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
     EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
   }
-
-  TU.AdditionalFiles["a.h"] = "";
-  TU.AdditionalFiles["b.h"] = "";
-  TU.AdditionalFiles["c.h"] = "";
-  // Make sure replay logic works with patched preambles.
-  llvm::StringLiteral Baseline = R"cpp(
-    #include "a.h"
-    #include "c.h")cpp";
-  MockFS FS;
-  TU.Code = Baseline.str();
-  auto Inputs = TU.inputs(FS);
-  auto BaselinePreamble = TU.preamble();
-  ASSERT_TRUE(BaselinePreamble);
-
-  // First make sure we don't crash on various modifications to the preamble.
-  llvm::StringLiteral Cases[] = {
-      // clang-format off
-      // New include in middle.
-      R"cpp(
-        #include "a.h"
-        #include "b.h"
-        #include "c.h")cpp",
-      // New include at top.
-      R"cpp(
-        #include "b.h"
-        #include "a.h"
-        #include "c.h")cpp",
-      // New include at bottom.
-      R"cpp(
-        #include "a.h"
-        #include "c.h"
-        #include "b.h")cpp",
-      // Same size with a missing include.
-      R"cpp(
-        #include "a.h"
-        #include "b.h")cpp",
-      // Smaller with no new includes.
-      R"cpp(
-        #include "a.h")cpp",
-      // Smaller with a new includes.
-      R"cpp(
-        #include "b.h")cpp",
-      // clang-format on
-  };
-  for (llvm::StringLiteral Case : Cases) {
-    TU.Code = Case.str();
-
-    IgnoreDiagnostics Diags;
-    auto CI = buildCompilerInvocation(TU.inputs(FS), Diags);
-    auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
-                                       std::move(CI), {}, BaselinePreamble);
-    ASSERT_TRUE(PatchedAST);
-    EXPECT_FALSE(PatchedAST->getDiagnostics());
-  }
-
-  // Then ensure correctness by making sure includes were seen only once.
-  // Note that we first see the includes from the patch, as preamble includes
-  // are replayed after exiting the built-in file.
-  Includes.clear();
-  TU.Code = R"cpp(
-    #include "a.h"
-    #include "b.h")cpp";
-  IgnoreDiagnostics Diags;
-  auto CI = buildCompilerInvocation(TU.inputs(FS), Diags);
-  auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
-                                     std::move(CI), {}, BaselinePreamble);
-  ASSERT_TRUE(PatchedAST);
-  EXPECT_FALSE(PatchedAST->getDiagnostics());
-  EXPECT_THAT(Includes,
-              ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
-                          WithFileName("b.h"), WithFileName("a.h")));
 }
 
 TEST(ParsedASTTest, PatchesAdditionalIncludes) {
   llvm::StringLiteral ModifiedContents = R"cpp(
     #include "baz.h"
     #include "foo.h"
     #include "sub/aux.h"
     void bar() {