From f6b1323bc680812e04904293854c356530985bcd Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 1 Oct 2020 16:14:31 +0200 Subject: [PATCH] Reland [clangd] clangd --check: standalone diagnosis of common problems This reverts commit 30d07b14a274f075a01d201ad59723ca1a4a9b57. Test failures have (hopefully) been fixed. --- clang-tools-extra/clangd/test/check-fail.test | 14 + 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, 316 insertions(+), 3 deletions(-) create mode 100644 clang-tools-extra/clangd/test/check-fail.test create mode 100644 clang-tools-extra/clangd/test/check.test create 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 new file mode 100644 index 000000000000..0ee777f02cc5 --- /dev/null +++ b/clang-tools-extra/clangd/test/check-fail.test @@ -0,0 +1,14 @@ +// RUN: cp %s %t.cpp +// RUN: not clangd -check=%t.cpp 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 $ ) 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 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 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 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", @@ -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(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(ErrorResultCode::CantRunAsXPCService); #endif } else { log("Starting LSP over stdin/stdout");