njames93 updated this revision to Diff 307615.
njames93 added a comment.
Removed duplicated logging of options.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91029/new/
https://reviews.llvm.org/D91029
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/TidyProvider.h
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#include "../TidyProvider.h"
#include "Compiler.h"
#include "ParsedAST.h"
#include "TestFS.h"
@@ -58,8 +59,7 @@
// Extra arguments for the compiler invocation.
std::vector<std::string> ExtraArgs;
- llvm::Optional<std::string> ClangTidyChecks;
- llvm::Optional<std::string> ClangTidyWarningsAsErrors;
+ TidyProvider ClangTidyProvider = {};
// Index to use when building AST.
const SymbolIndex *ExternalIndex = nullptr;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -59,8 +59,8 @@
FS.OverlayRealFileSystemForModules = true;
Inputs.TFS = &FS;
Inputs.Opts = ParseOptions();
- Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
- Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
+ if (ClangTidyProvider)
+ Inputs.ClangTidyProvider = ClangTidyProvider;
Inputs.Index = ExternalIndex;
if (Inputs.Index)
Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
#include "SourceCode.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "TidyProvider.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
@@ -250,7 +251,7 @@
TestTU TU;
// this check runs the preprocessor, we need to make sure it does not break
// our recording logic.
- TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+ TU.ClangTidyProvider = addTidyChecks("modernize-use-trailing-return-type");
TU.Code = "inline int foo() {}";
auto AST = TU.build();
@@ -406,7 +407,7 @@
"replay-preamble-module", "");
TestTU TU;
// This check records inclusion directives replayed by clangd.
- TU.ClangTidyChecks = "replay-preamble-check";
+ TU.ClangTidyProvider = addTidyChecks("replay-preamble-check");
llvm::Annotations Test(R"cpp(
$hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
$hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -14,6 +14,7 @@
#include "TestFS.h"
#include "TestIndex.h"
#include "TestTU.h"
+#include "TidyProvider.h"
#include "index/MemIndex.h"
#include "support/Path.h"
#include "clang/Basic/Diagnostic.h"
@@ -129,7 +130,7 @@
}
)cpp");
auto TU = TestTU::withCode(Test.code());
- TU.ClangTidyChecks = "-*,google-explicit-constructor";
+ TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
EXPECT_THAT(
TU.build().getDiagnostics(),
ElementsAre(
@@ -201,8 +202,8 @@
auto TU = TestTU::withCode(Test.code());
// Enable alias clang-tidy checks, these check emits the same diagnostics
// (except the check name).
- TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, "
- "hicpp-uppercase-literal-suffix";
+ TU.ClangTidyProvider = addTidyChecks("readability-uppercase-literal-suffix,"
+ "hicpp-uppercase-literal-suffix");
// Verify that we filter out the duplicated diagnostic message.
EXPECT_THAT(
TU.build().getDiagnostics(),
@@ -245,9 +246,10 @@
)cpp");
auto TU = TestTU::withCode(Test.code());
TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
- TU.ClangTidyChecks =
- "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, "
- "modernize-deprecated-headers, modernize-use-trailing-return-type";
+ TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression,"
+ "bugprone-macro-repeated-side-effects,"
+ "modernize-deprecated-headers,"
+ "modernize-use-trailing-return-type");
EXPECT_THAT(
TU.build().getDiagnostics(),
UnorderedElementsAre(
@@ -289,7 +291,7 @@
auto TU = TestTU::withCode(Test.code());
TU.ExtraArgs = {"-isystem."};
TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = "";
- TU.ClangTidyChecks = "-*, llvm-include-order";
+ TU.ClangTidyProvider = addTidyChecks("llvm-include-order");
EXPECT_THAT(
TU.build().getDiagnostics(),
Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"),
@@ -361,7 +363,7 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "modernize-loop-convert";
+ TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert");
EXPECT_THAT(
TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
@@ -384,7 +386,7 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "bugprone-integer-division";
+ TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division");
EXPECT_THAT(
TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
@@ -401,8 +403,8 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "bugprone-integer-division";
- TU.ClangTidyWarningsAsErrors = "bugprone-integer-division";
+ TU.ClangTidyProvider =
+ addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
EXPECT_THAT(
TU.build().getDiagnostics(),
UnorderedElementsAre(::testing::AllOf(
@@ -452,8 +454,8 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "bugprone-integer-division";
- TU.ClangTidyWarningsAsErrors = "bugprone-integer-division";
+ TU.ClangTidyProvider =
+ addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
}
@@ -468,7 +470,7 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+ TU.ClangTidyProvider = addTidyChecks("bugprone-bad-signal-to-kill-thread");
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
}
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -16,6 +16,7 @@
#include "SyncAPI.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "TidyProvider.h"
#include "URI.h"
#include "support/MemoryTree.h"
#include "support/Path.h"
@@ -1215,16 +1216,19 @@
} DiagConsumer;
MockFS FS;
+ // These checks don't work well in clangd, even if configured they shouldn't
+ // run.
+ FS.Files[testPath(".clang-tidy")] = R"(
+ Checks: -*,bugprone-use-after-move,llvm-header-guard
+ )";
MockCompilationDatabase CDB;
+ std::vector<TidyProvider> Stack;
+ Stack.push_back(provideClangTidyFiles(FS));
+ Stack.push_back(disableUnusableChecks());
+ TidyProvider Provider = combine(std::move(Stack));
CDB.ExtraClangFlags = {"-xc++"};
auto Opts = ClangdServer::optsForTest();
- Opts.GetClangTidyOptions = [](llvm::vfs::FileSystem &, llvm::StringRef) {
- auto Opts = tidy::ClangTidyOptions::getDefaults();
- // These checks don't work well in clangd, even if configured they shouldn't
- // run.
- Opts.Checks = "bugprone-use-after-move,llvm-header-guard";
- return Opts;
- };
+ Opts.ClangTidyProvider = Provider;
ClangdServer Server(CDB, FS, Opts, &DiagConsumer);
const char *SourceContents = R"cpp(
struct Foo { Foo(); Foo(Foo&); Foo(Foo&&); };
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -11,6 +11,7 @@
#include "Features.inc"
#include "PathMapping.h"
#include "Protocol.h"
+#include "TidyProvider.h"
#include "Transport.h"
#include "index/Background.h"
#include "index/Index.h"
@@ -837,35 +838,21 @@
}
// Create an empty clang-tidy option.
- std::mutex ClangTidyOptMu;
- std::unique_ptr<tidy::ClangTidyOptionsProvider>
- ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/
+ TidyProvider ClangTidyOptProvider;
if (EnableClangTidy) {
- auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults();
- EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
- EmptyDefaults.User = llvm::sys::Process::GetEnv("USER");
-#ifdef _WIN32
- if (!EmptyDefaults.User)
- EmptyDefaults.User = llvm::sys::Process::GetEnv("USERNAME");
-#endif
- tidy::ClangTidyOptions OverrideClangTidyOptions;
+ std::vector<TidyProvider> Providers;
+ Providers.reserve(4 + EnableConfig);
+ Providers.push_back(provideEnvironment());
+ Providers.push_back(provideClangTidyFiles(TFS));
+ if (EnableConfig)
+ Providers.push_back(provideClangdConfig());
if (!ClangTidyChecks.empty())
- OverrideClangTidyOptions.Checks = ClangTidyChecks;
- ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
- tidy::ClangTidyGlobalOptions(),
- /* Default */ EmptyDefaults,
- /* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None));
- Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
- llvm::StringRef File) {
- // This function must be thread-safe and tidy option providers are not.
- tidy::ClangTidyOptions Opts;
- {
- std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
- // FIXME: use the FS provided to the function.
- Opts = ClangTidyOptProvider->getOptions(File);
- }
- return Opts;
- };
+ Providers.push_back(addTidyChecks(ClangTidyChecks));
+ else
+ Providers.push_back(provideDefaultChecks());
+ Providers.push_back(disableUnusableChecks());
+ ClangTidyOptProvider = combine(std::move(Providers));
+ Opts.ClangTidyProvider = ClangTidyOptProvider;
}
Opts.AsyncPreambleBuilds = AsyncPreamble;
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -133,8 +133,6 @@
return false;
}
}
- Inputs.Opts.ClangTidyOpts =
- Opts.GetClangTidyOptions(*TFS.view(llvm::None), File);
log("Parsing command...");
Invocation =
buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args);
Index: clang-tools-extra/clangd/TidyProvider.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/TidyProvider.h
@@ -0,0 +1,64 @@
+//===--- TidyProvider.h - create options for running clang-tidy------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H
+
+#include "../clang-tidy/ClangTidyOptions.h"
+#include "support/ThreadsafeFS.h"
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/STLExtras.h"
+
+namespace clang {
+namespace clangd {
+
+/// A factory to modify a \ref tidy::ClangTidyOptions
+using TidyProvider = llvm::unique_function<void(tidy::ClangTidyOptions &,
+ /*Filename=*/llvm::StringRef,
+ /*CWD*/ PathRef) const>;
+
+/// A factory to modify a \ref tidy::ClangTidyOptions that doesn't store any
+/// state.
+using TidyProviderRef = llvm::function_ref<void(tidy::ClangTidyOptions &,
+ /*Filename=*/llvm::StringRef,
+ /*CWD*/ PathRef)>;
+
+TidyProvider combine(std::vector<TidyProvider> Providers);
+
+/// Provider that just sets the defaults.
+TidyProviderRef provideEnvironment();
+
+/// Provider that will enable a nice set of default checks if none are
+/// specified.
+TidyProviderRef provideDefaultChecks();
+
+/// Provider the enables a specific set of checks and warnings as errors.
+TidyProvider addTidyChecks(llvm::StringRef Checks,
+ llvm::StringRef WarningsAsErrors = {});
+
+/// Provider that will disable checks known to not work with clangd. \p
+/// ExtraBadChecks specifies any other checks that should be always
+/// disabled.
+TidyProvider
+disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks = {});
+
+/// Provider that searches for .clang-tidy configuration files in the directory
+/// tree.
+TidyProvider provideClangTidyFiles(ThreadsafeFS &);
+
+// Provider that uses clangd configuration files.
+TidyProviderRef provideClangdConfig();
+
+tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
+ llvm::StringRef Filename,
+ PathRef CWD);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_TIDYPROVIDER_H
Index: clang-tools-extra/clangd/TidyProvider.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -0,0 +1,235 @@
+//===--- TidyProvider.cpp - create options for running clang-tidy----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "TidyProvider.h"
+#include "Config.h"
+#include "support/Logger.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Process.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
+
+namespace clang {
+namespace clangd {
+
+static void mergeCheckList(llvm::Optional<std::string> &Checks,
+ llvm::StringRef List) {
+ if (List.empty())
+ return;
+ if (!Checks || Checks->empty()) {
+ Checks.emplace(List);
+ return;
+ }
+ *Checks = llvm::join_items(",", *Checks, List);
+}
+
+static llvm::Optional<tidy::ClangTidyOptions>
+tryReadConfigFile(llvm::vfs::FileSystem *FS, llvm::StringRef Directory) {
+ assert(!Directory.empty());
+ // We guaranteed that child directories of Directory exist, so this assert
+ // should hopefully never fail.
+ assert(FS->exists(Directory));
+
+ llvm::SmallString<128> ConfigFile(Directory);
+ llvm::sys::path::append(ConfigFile, ".clang-tidy");
+
+ llvm::ErrorOr<llvm::vfs::Status> FileStatus = FS->status(ConfigFile);
+
+ if (!FileStatus || !FileStatus->isRegularFile())
+ return llvm::None;
+
+ llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
+ FS->getBufferForFile(ConfigFile);
+ if (std::error_code EC = Text.getError()) {
+ elog("Can't read '{0}': {1}", ConfigFile, EC.message());
+ return llvm::None;
+ }
+
+ // Skip empty files, e.g. files opened for writing via shell output
+ // redirection.
+ if ((*Text)->getBuffer().empty())
+ return llvm::None;
+ llvm::ErrorOr<tidy::ClangTidyOptions> ParsedOptions =
+ tidy::parseConfiguration((*Text)->getBuffer());
+ if (!ParsedOptions) {
+ if (ParsedOptions.getError())
+ elog("Error parsing clang-tidy configuration in '{0}': {1}", ConfigFile,
+ ParsedOptions.getError().message());
+ return llvm::None;
+ }
+ return std::move(*ParsedOptions);
+}
+
+TidyProviderRef provideEnvironment() {
+ static const llvm::Optional<std::string> User = [] {
+ llvm::Optional<std::string> Ret = llvm::sys::Process::GetEnv("USER");
+#ifdef _WIN32
+ if (!Ret)
+ return llvm::sys::Process::GetEnv("USERNAME");
+#endif
+ return Ret;
+ }();
+
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, PathRef) {
+ if (User)
+ Opts.User = User;
+ };
+}
+
+TidyProviderRef provideDefaultChecks() {
+ // These default checks are chosen for:
+ // - low false-positive rate
+ // - providing a lot of value
+ // - being reasonably efficient
+ static const std::string DefaultChecks = llvm::join_items(
+ ",", "readability-misleading-indentation", "readability-deleted-default",
+ "bugprone-integer-division", "bugprone-sizeof-expression",
+ "bugprone-suspicious-missing-comma", "bugprone-unused-raii",
+ "bugprone-unused-return-value", "misc-unused-using-decls",
+ "misc-unused-alias-decls", "misc-definitions-in-headers");
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, PathRef) {
+ if (!Opts.Checks || Opts.Checks->empty())
+ Opts.Checks = DefaultChecks;
+ };
+}
+
+TidyProvider addTidyChecks(llvm::StringRef Checks,
+ llvm::StringRef WarningsAsErrors) {
+ return [Checks = std::string(Checks),
+ WarningsAsErrors = std::string(WarningsAsErrors)](
+ tidy::ClangTidyOptions &Opts, llvm::StringRef, PathRef) {
+ mergeCheckList(Opts.Checks, Checks);
+ mergeCheckList(Opts.WarningsAsErrors, WarningsAsErrors);
+ };
+}
+
+TidyProvider disableUnusableChecks(llvm::ArrayRef<std::string> ExtraBadChecks) {
+ constexpr llvm::StringLiteral Seperator(",");
+ static const std::string BadChecks =
+ llvm::join_items(Seperator,
+ // We want this list to start with a seperator to
+ // simplify appending in the lambda. So including an
+ // empty string here will force that.
+ "",
+ // ----- False Positives -----
+
+ // Check relies on seeing ifndef/define/endif directives,
+ // clangd doesn't replay those when using a preamble.
+ "-llvm-header-guard",
+
+ // ----- Crashing Checks -----
+
+ // Check can choke on invalid (intermediate) c++
+ // code, which is often the case when clangd
+ // tries to build an AST.
+ "-bugprone-use-after-move");
+
+ size_t Size = BadChecks.size();
+ for (const std::string &Str : ExtraBadChecks) {
+ if (Str.empty())
+ continue;
+ Size += Seperator.size();
+ if (LLVM_LIKELY(Str.front() != '-'))
+ ++Size;
+ Size += Str.size();
+ }
+ std::string DisableGlob;
+ DisableGlob.reserve(Size);
+ DisableGlob += BadChecks;
+ for (const std::string &Str : ExtraBadChecks) {
+ if (Str.empty())
+ continue;
+ DisableGlob += Seperator;
+ if (LLVM_LIKELY(Str.front() != '-'))
+ DisableGlob.push_back('-');
+ DisableGlob += Str;
+ }
+
+ return [DisableList(std::move(DisableGlob))](tidy::ClangTidyOptions &Opts,
+ llvm::StringRef, PathRef) {
+ if (Opts.Checks && !Opts.Checks->empty())
+ Opts.Checks->append(DisableList);
+ };
+}
+
+TidyProviderRef provideClangdConfig() {
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, PathRef) {
+ const auto &CurTidyConfig = Config::current().ClangTidy;
+ if (!CurTidyConfig.Checks.empty())
+ mergeCheckList(Opts.Checks, CurTidyConfig.Checks);
+
+ for (const auto &CheckOption : CurTidyConfig.CheckOptions)
+ Opts.CheckOptions.insert_or_assign(CheckOption.getKey(),
+ tidy::ClangTidyOptions::ClangTidyValue(
+ CheckOption.getValue(), 10000U));
+ };
+}
+
+TidyProvider provideClangTidyFiles(ThreadsafeFS &TFS) {
+ return [&TFS](tidy::ClangTidyOptions &Opts, llvm::StringRef Filename,
+ PathRef CWD) {
+ llvm::SmallVector<tidy::ClangTidyOptions, 4> OptionStack;
+ auto FS(CWD.empty() ? TFS.view(llvm::None) : TFS.view(CWD));
+ llvm::SmallString<256> AbsolutePath(Filename);
+
+ if (FS->makeAbsolute(AbsolutePath))
+ return;
+
+ llvm::sys::path::remove_dots(AbsolutePath, true);
+ llvm::StringRef Directory = llvm::sys::path::parent_path(AbsolutePath);
+ {
+ auto Status = FS->status(Directory);
+
+ if (!Status || !Status->isDirectory()) {
+ elog("Error reading configuration from {0}: directory doesn't exist",
+ Directory);
+ return;
+ }
+ }
+
+ // FIXME: Store options in a cache that validates itself against changes
+ // during the clangd session.
+ for (llvm::StringRef CurrentDirectory = Directory;
+ !CurrentDirectory.empty();
+ CurrentDirectory = llvm::sys::path::parent_path(CurrentDirectory)) {
+ auto ConfigFile = tryReadConfigFile(FS.get(), CurrentDirectory);
+ if (!ConfigFile)
+ continue;
+ OptionStack.push_back(std::move(*ConfigFile));
+ // Should we search for a parent config to merge
+ if (!OptionStack.back().InheritParentConfig.getValueOr(false))
+ break;
+ }
+ unsigned Order = 1U;
+ for (auto &Option : llvm::reverse(OptionStack))
+ Opts.mergeWith(Option, Order++);
+ };
+}
+
+TidyProvider combine(std::vector<TidyProvider> Providers) {
+ return
+ [Providers(std::move(Providers))](tidy::ClangTidyOptions &Opts,
+ llvm::StringRef Filename, PathRef CWD) {
+ for (const auto &Provider : Providers)
+ Provider(Opts, Filename, CWD);
+ };
+}
+
+tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
+ llvm::StringRef Filename,
+ PathRef CWD) {
+ tidy::ClangTidyOptions Opts = tidy::ClangTidyOptions::getDefaults();
+ Opts.Checks->clear();
+ if (Provider)
+ Provider(Opts, Filename, CWD);
+ return Opts;
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -17,6 +17,7 @@
#include "IncludeFixer.h"
#include "Preamble.h"
#include "SourceCode.h"
+#include "TidyProvider.h"
#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "support/Logger.h"
@@ -292,13 +293,15 @@
llvm::Optional<tidy::ClangTidyContext> CTContext;
{
trace::Span Tracer("ClangTidyInit");
+ tidy::ClangTidyOptions ClangTidyOpts = getTidyOptionsForFile(
+ Inputs.ClangTidyProvider, Filename, Inputs.CompileCommand.Directory);
dlog("ClangTidy configuration for file {0}: {1}", Filename,
- tidy::configurationAsText(Inputs.Opts.ClangTidyOpts));
+ tidy::configurationAsText(ClangTidyOpts));
tidy::ClangTidyCheckFactories CTFactories;
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
E.instantiate()->addCheckFactories(CTFactories);
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
- tidy::ClangTidyGlobalOptions(), Inputs.Opts.ClangTidyOpts));
+ tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(Filename);
Index: clang-tools-extra/clangd/Compiler.h
===================================================================
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -15,8 +15,8 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
-#include "../clang-tidy/ClangTidyOptions.h"
#include "GlobalCompilationDatabase.h"
+#include "TidyProvider.h"
#include "index/Index.h"
#include "support/ThreadsafeFS.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -37,7 +37,6 @@
// Options to run clang e.g. when parsing AST.
struct ParseOptions {
- tidy::ClangTidyOptions ClangTidyOpts;
bool SuggestMissingIncludes = false;
bool BuildRecoveryAST = false;
bool PreserveRecoveryASTType = false;
@@ -56,6 +55,7 @@
// Used to recover from diagnostics (e.g. find missing includes for symbol).
const SymbolIndex *Index = nullptr;
ParseOptions Opts = ParseOptions();
+ TidyProviderRef ClangTidyProvider = {};
};
/// Builds compiler invocation that could be used to build AST or preamble.
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -41,13 +41,6 @@
namespace clang {
namespace clangd {
-
-/// When set, used by ClangdServer to get clang-tidy options for each particular
-/// file. Must be thread-safe. We use this instead of ClangTidyOptionsProvider
-/// to allow reading tidy configs from the VFS used for parsing.
-using ClangTidyOptionsBuilder = std::function<tidy::ClangTidyOptions(
- llvm::vfs::FileSystem &, llvm::StringRef /*File*/)>;
-
/// Manages a collection of source files and derived data (ASTs, indexes),
/// and provides language-aware features such as code completion.
///
@@ -121,12 +114,9 @@
/// If set, queried to obtain the configuration to handle each request.
config::Provider *ConfigProvider = nullptr;
- /// If set, enable clang-tidy in clangd and use to it get clang-tidy
- /// configurations for a particular file.
- /// Clangd supports only a small subset of ClangTidyOptions, these options
- /// (Checks, CheckOptions) are about which clang-tidy checks will be
- /// enabled.
- ClangTidyOptionsBuilder GetClangTidyOptions;
+ /// The Options provider to use when running clang-tidy. If null, clang-tidy
+ /// checks will be disabled.
+ TidyProviderRef ClangTidyProvider;
/// If true, force -frecovery-ast flag.
/// If false, respect the value in clang.
@@ -389,7 +379,7 @@
std::vector<std::unique_ptr<SymbolIndex>> MergedIdx;
// When set, provides clang-tidy options for a specific file.
- ClangTidyOptionsBuilder GetClangTidyOptions;
+ TidyProviderRef ClangTidyProvider;
// If this is true, suggest include insertion fixes for diagnostic errors that
// can be caused by missing includes (e.g. member access in incomplete type).
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -114,40 +114,6 @@
bool TheiaSemanticHighlighting;
};
-// Set of clang-tidy checks that don't work well in clangd, either due to
-// crashes or false positives.
-// Returns a clang-tidy filter string: -check1,-check2.
-llvm::StringRef getUnusableTidyChecks() {
- static const std::string FalsePositives =
- llvm::join_items(", ",
- // Check relies on seeing ifndef/define/endif directives,
- // clangd doesn't replay those when using a preamble.
- "-llvm-header-guard");
- static const std::string CrashingChecks =
- llvm::join_items(", ",
- // Check can choke on invalid (intermediate) c++ code,
- // which is often the case when clangd tries to build an
- // AST.
- "-bugprone-use-after-move");
- static const std::string UnusableTidyChecks =
- llvm::join_items(", ", FalsePositives, CrashingChecks);
- return UnusableTidyChecks;
-}
-
-// Returns a clang-tidy options string: check1,check2.
-llvm::StringRef getDefaultTidyChecks() {
- // These default checks are chosen for:
- // - low false-positive rate
- // - providing a lot of value
- // - being reasonably efficient
- static const std::string DefaultChecks = llvm::join_items(
- ",", "readability-misleading-indentation", "readability-deleted-default",
- "bugprone-integer-division", "bugprone-sizeof-expression",
- "bugprone-suspicious-missing-comma", "bugprone-unused-raii",
- "bugprone-unused-return-value", "misc-unused-using-decls",
- "misc-unused-alias-decls", "misc-definitions-in-headers");
- return DefaultChecks;
-}
} // namespace
ClangdServer::Options ClangdServer::optsForTest() {
@@ -178,7 +144,7 @@
? new FileIndex(Opts.HeavyweightDynamicSymbolIndex,
Opts.CollectMainFileRefs)
: nullptr),
- GetClangTidyOptions(Opts.GetClangTidyOptions),
+ ClangTidyProvider(Opts.ClangTidyProvider),
SuggestMissingIncludes(Opts.SuggestMissingIncludes),
BuildRecoveryAST(Opts.BuildRecoveryAST),
PreserveRecoveryASTType(Opts.PreserveRecoveryASTType),
@@ -236,20 +202,6 @@
llvm::StringRef Version,
WantDiagnostics WantDiags, bool ForceRebuild) {
ParseOptions Opts;
- Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
- // FIXME: call tidy options builder on the worker thread, it can do IO.
- if (GetClangTidyOptions)
- Opts.ClangTidyOpts =
- GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File);
- if (Opts.ClangTidyOpts.Checks.hasValue()) {
- // If the set of checks was configured, make sure clangd incompatible ones
- // are disabled.
- Opts.ClangTidyOpts.Checks = llvm::join_items(
- ", ", *Opts.ClangTidyOpts.Checks, getUnusableTidyChecks());
- } else {
- // Otherwise provide a nice set of defaults.
- Opts.ClangTidyOpts.Checks = getDefaultTidyChecks().str();
- }
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
// Compile command is set asynchronously during update, as it can be slow.
@@ -260,6 +212,7 @@
Inputs.ForceRebuild = ForceRebuild;
Inputs.Opts = std::move(Opts);
Inputs.Index = Index;
+ Inputs.ClangTidyProvider = ClangTidyProvider;
Inputs.Opts.BuildRecoveryAST = BuildRecoveryAST;
Inputs.Opts.PreserveRecoveryASTType = PreserveRecoveryASTType;
bool NewFile = WorkScheduler.update(File, Inputs, WantDiags);
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -81,6 +81,7 @@
SemanticSelection.cpp
SourceCode.cpp
QueryDriverDatabase.cpp
+ TidyProvider.cpp
TUScheduler.cpp
URI.cpp
XRefs.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
