From 30d07b14a274f075a01d201ad59723ca1a4a9b57 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 1 Oct 2020 16:10:03 +0200 Subject: [PATCH] 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. --- clang-tools-extra/clangd/test/check-fail.test | 13 -- clang-tools-extra/clangd/test/check.test | 13 -- clang-tools-extra/clangd/tool/CMakeLists.txt | 1 - clang-tools-extra/clangd/tool/Check.cpp | 258 -------------------------- clang-tools-extra/clangd/tool/ClangdMain.cpp | 33 +--- 5 files changed, 3 insertions(+), 315 deletions(-) delete mode 100644 clang-tools-extra/clangd/test/check-fail.test delete mode 100644 clang-tools-extra/clangd/test/check.test delete mode 100644 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 7462ce5..0000000 --- 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 832629c..0000000 --- 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 65e0aa3..670e5a1 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 $ ) diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp deleted file mode 100644 index 14ee0fd..0000000 --- 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 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 Invocation; - format::FormatStyle Style; - // from buildAST - std::shared_ptr Preamble; - llvm::Optional 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 BaseCDB = - std::make_unique( - 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( - BaseCDB.get(), std::vector{}, - 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 Contents) { - StoreDiags CaptureInvocationDiags; - std::vector 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 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{}, 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 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 - #include - - 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 98daaf9..a897a9a 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 Test{ Hidden, }; -opt 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=, attempts to parse a particular file."), - init(""), - ValueOptional, -}; - enum PCHStorageFlag { Disk, Memory }; opt 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(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(ErrorResultCode::CantRunAsXPCService); + return (int)ErrorResultCode::CantRunAsXPCService; #endif } else { log("Starting LSP over stdin/stdout"); -- 2.7.4