[clang-tools-extra] 79fbcbf - [clangd] clangd --check: standalone diagnosis of common problems

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 1 06:47:56 PDT 2020


Author: Sam McCall
Date: 2020-10-01T15:47:47+02:00
New Revision: 79fbcbff41734e3d07e6200d33c3e40732dfae6a

URL: https://github.com/llvm/llvm-project/commit/79fbcbff41734e3d07e6200d33c3e40732dfae6a
DIFF: https://github.com/llvm/llvm-project/commit/79fbcbff41734e3d07e6200d33c3e40732dfae6a.diff

LOG: [clangd] clangd --check: standalone diagnosis of common problems

This is a tool to simply parse a file as clangd would, and run some
common features (code actions, go-to-definition, hover) in an attempt to
trigger or reproduce crashes, error diagnostics, etc.

This is easier and more predictable than loading the file in clangd, because:
 - there's no editor/plugin variation to worry about
 - there's no accidental variation of user behavior or other extraneous requests
 - we trigger features at every token, rather than guessing
 - everything is synchronoous, logs are easier to reason about
 - it's easier to (get users to) capture logs when running on the command-line

This is a fairly lightweight variant of this idea.
We could do a lot more with it, and maybe we should.
But I can't in the near future, and experience will tell us if we made
the right tradeoffs and if it's worth investing further.

Differential Revision: https://reviews.llvm.org/D88338

Added: 
    clang-tools-extra/clangd/test/check-fail.test
    clang-tools-extra/clangd/test/check.test
    clang-tools-extra/clangd/tool/Check.cpp

Modified: 
    clang-tools-extra/clangd/tool/CMakeLists.txt
    clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/test/check-fail.test b/clang-tools-extra/clangd/test/check-fail.test
new file mode 100644
index 000000000000..7462ce5ecf5f
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check-fail.test
@@ -0,0 +1,13 @@
+// RUN: not clangd -check=%s 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: Testing on source file {{.*}}check-fail.test
+// CHECK: internal (cc1) args are: -cc1
+// CHECK: Building preamble...
+// CHECK: [pp_file_not_found] Line {{.*}}: 'missing.h' file not found
+// CHECK: Building AST...
+// CHECK: Testing features at each token
+// CHECK: tweak: ExpandAutoType ==> FAIL
+// CHECK: All checks completed, 2 errors
+
+#include "missing.h"
+auto x = []{};

diff  --git a/clang-tools-extra/clangd/test/check.test b/clang-tools-extra/clangd/test/check.test
new file mode 100644
index 000000000000..832629ce29ef
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check.test
@@ -0,0 +1,13 @@
+# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s
+
+CHECK: Testing on source file {{.*}}test.cc
+CHECK: internal (cc1) args are: -cc1
+CHECK: Building preamble...
+CHECK: Built preamble
+CHECK: Building AST...
+CHECK: Testing features at each token
+CHECK-DAG: hover: false
+CHECK-DAG: hover: true
+CHECK-DAG: tweak: AddUsing
+CHECK: All checks completed, 0 errors
+

diff  --git a/clang-tools-extra/clangd/tool/CMakeLists.txt b/clang-tools-extra/clangd/tool/CMakeLists.txt
index 670e5a17013a..65e0aa35f265 100644
--- a/clang-tools-extra/clangd/tool/CMakeLists.txt
+++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -3,6 +3,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  Check.cpp
   $<TARGET_OBJECTS:obj.clangDaemonTweaks>
   )
 

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
new file mode 100644
index 000000000000..14ee0fdec9c9
--- /dev/null
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -0,0 +1,258 @@
+//===--- Check.cpp - clangd self-diagnostics ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Many basic problems can occur processing a file in clangd, e.g.:
+//  - system includes are not found
+//  - crash when indexing its AST
+// clangd --check provides a simplified, isolated way to reproduce these,
+// with no editor, LSP, threads, background indexing etc to contend with.
+//
+// One important use case is gathering information for bug reports.
+// Another is reproducing crashes, and checking which setting prevent them.
+//
+// It simulates opening a file (determining compile command, parsing, indexing)
+// and then running features at many locations.
+//
+// Currently it adds some basic logging of progress and results.
+// We should consider extending it to also recognize common symptoms and
+// recommend solutions (e.g. standard library installation issues).
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangdLSPServer.h"
+#include "CodeComplete.h"
+#include "GlobalCompilationDatabase.h"
+#include "Hover.h"
+#include "ParsedAST.h"
+#include "Preamble.h"
+#include "SourceCode.h"
+#include "XRefs.h"
+#include "index/CanonicalIncludes.h"
+#include "index/FileIndex.h"
+#include "refactor/Tweak.h"
+#include "support/ThreadsafeFS.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Print (and count) the error-level diagnostics (warnings are ignored).
+unsigned showErrors(llvm::ArrayRef<Diag> Diags) {
+  unsigned ErrCount = 0;
+  for (const auto &D : Diags) {
+    if (D.Severity >= DiagnosticsEngine::Error) {
+      elog("[{0}] Line {1}: {2}", D.Name, D.Range.start.line + 1, D.Message);
+      ++ErrCount;
+    }
+  }
+  return ErrCount;
+}
+
+// This class is just a linear pipeline whose functions get called in sequence.
+// Each exercises part of clangd's logic on our test file and logs results.
+// Later steps depend on state built in earlier ones (such as the AST).
+// Many steps can fatally fail (return false), then subsequent ones cannot run.
+// Nonfatal failures are logged and tracked in ErrCount.
+class Checker {
+  // from constructor
+  std::string File;
+  ClangdLSPServer::Options Opts;
+  // from buildCommand
+  tooling::CompileCommand Cmd;
+  // from buildInvocation
+  ParseInputs Inputs;
+  std::unique_ptr<CompilerInvocation> Invocation;
+  format::FormatStyle Style;
+  // from buildAST
+  std::shared_ptr<const PreambleData> Preamble;
+  llvm::Optional<ParsedAST> AST;
+  FileIndex Index;
+
+public:
+  // Number of non-fatal errors seen.
+  unsigned ErrCount = 0;
+
+  Checker(llvm::StringRef File, const ClangdLSPServer::Options &Opts)
+      : File(File), Opts(Opts) {}
+
+  // Read compilation database and choose a compile command for the file.
+  bool buildCommand() {
+    log("Loading compilation database...");
+    std::unique_ptr<GlobalCompilationDatabase> BaseCDB =
+        std::make_unique<DirectoryBasedGlobalCompilationDatabase>(
+            Opts.CompileCommandsDir);
+    BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
+                                     std::move(BaseCDB));
+    auto Mangler = CommandMangler::detect();
+    if (Opts.ResourceDir)
+      Mangler.ResourceDir = *Opts.ResourceDir;
+    auto CDB = std::make_unique<OverlayCDB>(
+        BaseCDB.get(), std::vector<std::string>{},
+        tooling::ArgumentsAdjuster(std::move(Mangler)));
+
+    if (auto TrueCmd = CDB->getCompileCommand(File)) {
+      Cmd = std::move(*TrueCmd);
+      log("Compile command from CDB is: {0}", llvm::join(Cmd.CommandLine, " "));
+    } else {
+      Cmd = CDB->getFallbackCommand(File);
+      log("Generic fallback command is: {0}", llvm::join(Cmd.CommandLine, " "));
+    }
+
+    return true;
+  }
+
+  // Prepare inputs and build CompilerInvocation (parsed compile command).
+  bool buildInvocation(const ThreadsafeFS &TFS,
+                       llvm::Optional<std::string> Contents) {
+    StoreDiags CaptureInvocationDiags;
+    std::vector<std::string> CC1Args;
+    Inputs.CompileCommand = Cmd;
+    Inputs.TFS = &TFS;
+    if (Contents.hasValue()) {
+      Inputs.Contents = *Contents;
+      log("Imaginary source file contents:\n{0}", Inputs.Contents);
+    } else {
+      if (auto Contents = TFS.view(llvm::None)->getBufferForFile(File)) {
+        Inputs.Contents = Contents->get()->getBuffer().str();
+      } else {
+        elog("Couldn't read {0}: {1}", File, Contents.getError().message());
+        return false;
+      }
+    }
+    Inputs.Opts.ClangTidyOpts =
+        Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
+    log("Parsing command...");
+    Invocation =
+        buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args);
+    auto InvocationDiags = CaptureInvocationDiags.take();
+    ErrCount += showErrors(InvocationDiags);
+    log("internal (cc1) args are: {0}", llvm::join(CC1Args, " "));
+    if (!Invocation) {
+      elog("Failed to parse command line");
+      return false;
+    }
+
+    // FIXME: Check that resource-dir/built-in-headers exist?
+
+    Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
+
+    return true;
+  }
+
+  // Build preamble and AST, and index them.
+  bool buildAST() {
+    log("Building preamble...");
+    Preamble =
+        buildPreamble(File, *Invocation, Inputs, /*StoreInMemory=*/true,
+                      [&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP,
+                          const CanonicalIncludes &Includes) {
+                        if (!Opts.BuildDynamicSymbolIndex)
+                          return;
+                        log("Indexing headers...");
+                        Index.updatePreamble(File, /*Version=*/"null", Ctx,
+                                             std::move(PP), Includes);
+                      });
+    if (!Preamble) {
+      elog("Failed to build preamble");
+      return false;
+    }
+    ErrCount += showErrors(Preamble->Diags);
+
+    log("Building AST...");
+    AST = ParsedAST::build(File, Inputs, std::move(Invocation),
+                           /*InvocationDiags=*/std::vector<Diag>{}, Preamble);
+    if (!AST) {
+      elog("Failed to build AST");
+      return false;
+    }
+    ErrCount += showErrors(llvm::makeArrayRef(AST->getDiagnostics())
+                               .drop_front(Preamble->Diags.size()));
+
+    if (Opts.BuildDynamicSymbolIndex) {
+      log("Indexing AST...");
+      Index.updateMain(File, *AST);
+    }
+    return true;
+  }
+
+  // Run AST-based features at each token in the file.
+  void testLocationFeatures() {
+    log("Testing features at each token (may be slow in large files)");
+    auto SpelledTokens =
+        AST->getTokens().spelledTokens(AST->getSourceManager().getMainFileID());
+    for (const auto &Tok : SpelledTokens) {
+      unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
+      unsigned End = Start + Tok.length();
+      Position Pos = offsetToPosition(Inputs.Contents, Start);
+      // FIXME: dumping the tokens may leak sensitive code into bug reports.
+      // Add an option to turn this off, once we decide how options work.
+      vlog("  {0} {1}", Pos, Tok.text(AST->getSourceManager()));
+      auto Tree = SelectionTree::createRight(AST->getASTContext(),
+                                             AST->getTokens(), Start, End);
+      Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
+      for (const auto &T : prepareTweaks(Selection, Opts.TweakFilter)) {
+        auto Result = T->apply(Selection);
+        if (!Result) {
+          elog("    tweak: {0} ==> FAIL: {1}", T->id(), Result.takeError());
+          ++ErrCount;
+        } else {
+          vlog("    tweak: {0}", T->id());
+        }
+      }
+      unsigned Definitions = locateSymbolAt(*AST, Pos, &Index).size();
+      vlog("    definition: {0}", Definitions);
+
+      auto Hover = getHover(*AST, Pos, Style, &Index);
+      vlog("    hover: {0}", Hover.hasValue());
+
+      // FIXME: it'd be nice to include code completion, but it's too slow.
+      // Maybe in combination with a line restriction?
+    }
+  }
+};
+
+} // namespace
+
+bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
+           const ClangdLSPServer::Options &Opts) {
+  llvm::SmallString<0> FakeFile;
+  llvm::Optional<std::string> Contents;
+  if (File.empty()) {
+    llvm::sys::path::system_temp_directory(false, FakeFile);
+    llvm::sys::path::append(FakeFile, "test.cc");
+    File = FakeFile;
+    Contents = R"cpp(
+      #include <stddef.h>
+      #include <string>
+
+      size_t N = 50;
+      auto xxx = std::string(N, 'x');
+    )cpp";
+  }
+  log("Testing on source file {0}", File);
+
+  Checker C(File, Opts);
+  if (!C.buildCommand() || !C.buildInvocation(TFS, Contents) || !C.buildAST())
+    return false;
+  C.testLocationFeatures();
+
+  log("All checks completed, {0} errors", C.ErrCount);
+  return C.ErrCount == 0;
+}
+
+} // namespace clangd
+} // namespace clang

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index a897a9a3531d..98daaf957359 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -47,6 +47,11 @@
 
 namespace clang {
 namespace clangd {
+
+// Implemented in Check.cpp.
+bool check(const llvm::StringRef File, const ThreadsafeFS &TFS,
+           const ClangdLSPServer::Options &Opts);
+
 namespace {
 
 using llvm::cl::cat;
@@ -57,6 +62,7 @@ using llvm::cl::init;
 using llvm::cl::list;
 using llvm::cl::opt;
 using llvm::cl::OptionCategory;
+using llvm::cl::ValueOptional;
 using llvm::cl::values;
 
 // All flags must be placed in a category, or they will be shown neither in
@@ -354,6 +360,16 @@ opt<bool> Test{
     Hidden,
 };
 
+opt<Path> CheckFile{
+    "check",
+    cat(Misc),
+    desc("Parse one file in isolation instead of acting as a language server. "
+         "Useful to investigate/reproduce crashes or configuration problems. "
+         "With --check=<filename>, attempts to parse a particular file."),
+    init(""),
+    ValueOptional,
+};
+
 enum PCHStorageFlag { Disk, Memory };
 opt<PCHStorageFlag> PCHStorage{
     "pch-storage",
@@ -541,7 +557,8 @@ const char TestScheme::TestDir[] = "/clangd-test";
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
-  CantRunAsXPCService = 2
+  CantRunAsXPCService = 2,
+  CheckFailed = 3
 };
 
 int main(int argc, char *argv[]) {
@@ -646,7 +663,8 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
   // If a user ran `clangd` in a terminal without redirecting anything,
   // it's somewhat likely they're confused about how to use clangd.
   // Show them the help overview, which explains.
-  if (llvm::outs().is_displayed() && llvm::errs().is_displayed())
+  if (llvm::outs().is_displayed() && llvm::errs().is_displayed() &&
+      !CheckFile.getNumOccurrences())
     llvm::errs() << Overview << "\n";
   // Use buffered stream to stderr (we still flush each log message). Unbuffered
   // stream can cause significant (non-deterministic) latency for the logger.
@@ -825,6 +843,15 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
   // Shall we allow to customize the file limit?
   Opts.Rename.AllowCrossFile = CrossFileRename;
 
+  if (CheckFile.getNumOccurrences()) {
+    llvm::SmallString<256> Path;
+    llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/true);
+    log("Entering check mode (no LSP server)");
+    return check(Path, TFS, Opts)
+               ? 0
+               : static_cast<int>(ErrorResultCode::CheckFailed);
+  }
+
   // Initialize and run ClangdLSPServer.
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
@@ -835,7 +862,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
     TransportLayer = newXPCTransport();
 #else
     llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
-    return (int)ErrorResultCode::CantRunAsXPCService;
+    return static_cast<int>(ErrorResultCode::CantRunAsXPCService);
 #endif
   } else {
     log("Starting LSP over stdin/stdout");


        


More information about the cfe-commits mailing list