[clang-tools-extra] 30d07b1 - Revert "[clangd] clangd --check: standalone diagnosis of common problems"

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 1 07:10:32 PDT 2020


Author: Sam McCall
Date: 2020-10-01T16:10:03+02:00
New Revision: 30d07b14a274f075a01d201ad59723ca1a4a9b57

URL: https://github.com/llvm/llvm-project/commit/30d07b14a274f075a01d201ad59723ca1a4a9b57
DIFF: https://github.com/llvm/llvm-project/commit/30d07b14a274f075a01d201ad59723ca1a4a9b57.diff

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

This reverts commit 79fbcbff41734e3d07e6200d33c3e40732dfae6a.

The fallback command fails to parse for the test files if there's no
compile_commands.json in the tree.

Added: 
    

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

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


################################################################################
diff  --git a/clang-tools-extra/clangd/test/check-fail.test b/clang-tools-extra/clangd/test/check-fail.test
deleted file mode 100644
index 7462ce5ecf5f..000000000000
--- a/clang-tools-extra/clangd/test/check-fail.test
+++ /dev/null
@@ -1,13 +0,0 @@
-// 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
deleted file mode 100644
index 832629ce29ef..000000000000
--- a/clang-tools-extra/clangd/test/check.test
+++ /dev/null
@@ -1,13 +0,0 @@
-# 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 65e0aa35f265..670e5a17013a 100644
--- a/clang-tools-extra/clangd/tool/CMakeLists.txt
+++ b/clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -3,7 +3,6 @@ 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
deleted file mode 100644
index 14ee0fdec9c9..000000000000
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ /dev/null
@@ -1,258 +0,0 @@
-//===--- 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 98daaf957359..a897a9a3531d 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -47,11 +47,6 @@
 
 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;
@@ -62,7 +57,6 @@ 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
@@ -360,16 +354,6 @@ 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",
@@ -557,8 +541,7 @@ const char TestScheme::TestDir[] = "/clangd-test";
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
-  CantRunAsXPCService = 2,
-  CheckFailed = 3
+  CantRunAsXPCService = 2
 };
 
 int main(int argc, char *argv[]) {
@@ -663,8 +646,7 @@ 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() &&
-      !CheckFile.getNumOccurrences())
+  if (llvm::outs().is_displayed() && llvm::errs().is_displayed())
     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.
@@ -843,15 +825,6 @@ 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();
@@ -862,7 +835,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 static_cast<int>(ErrorResultCode::CantRunAsXPCService);
+    return (int)ErrorResultCode::CantRunAsXPCService;
 #endif
   } else {
     log("Starting LSP over stdin/stdout");


        


More information about the cfe-commits mailing list