njames93 updated this revision to Diff 306850.
njames93 added a comment.
Fix ChecksToDisable to actually disable the checks.
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;
+ mutable 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,8 @@
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 =
+ fixedTidyProvider("modernize-use-trailing-return-type");
TU.Code = "inline int foo() {}";
auto AST = TU.build();
@@ -406,7 +408,7 @@
"replay-preamble-module", "");
TestTU TU;
// This check records inclusion directives replayed by clangd.
- TU.ClangTidyChecks = "replay-preamble-check";
+ TU.ClangTidyProvider = fixedTidyProvider("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 = fixedTidyProvider("-*,google-explicit-constructor");
EXPECT_THAT(
TU.build().getDiagnostics(),
ElementsAre(
@@ -201,8 +202,9 @@
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 =
+ fixedTidyProvider("-*, readability-uppercase-literal-suffix, "
+ "hicpp-uppercase-literal-suffix");
// Verify that we filter out the duplicated diagnostic message.
EXPECT_THAT(
TU.build().getDiagnostics(),
@@ -245,9 +247,9 @@
)cpp");
auto TU = TestTU::withCode(Test.code());
TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
- TU.ClangTidyChecks =
+ TU.ClangTidyProvider = fixedTidyProvider(
"-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, "
- "modernize-deprecated-headers, modernize-use-trailing-return-type";
+ "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 = fixedTidyProvider("-*, 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 = fixedTidyProvider("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 = fixedTidyProvider("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 = fixedTidyProvider("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 = fixedTidyProvider("bugprone-integer-division",
+ "bugprone-integer-division");
EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
}
@@ -468,7 +470,8 @@
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
- TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+ TU.ClangTidyProvider =
+ fixedTidyProvider("-*,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"
@@ -1214,17 +1215,25 @@
bool HadDiagsInLastCallback = false;
} DiagConsumer;
+ // We need to use the actual options provider here as we're interested in
+ // making sure those providers correctly disable problematic checks.
+ // As an added bonus it demonstrates the provider is capable of handling the
+ // ThreadsafeFS.
+
MockFS FS;
+ FS.Files[testPath(".clang-tidy")] = R"(
+ Checks: -*,bugprone-use-after-move,llvm-header-guard
+ )";
MockCompilationDatabase CDB;
+ std::vector<TidyProvider> Stack;
+ Stack.push_back(provideEnvironment());
+ Stack.push_back(provideClangTidyFiles(FS));
+ Stack.push_back(provideDefaultChecks());
+ 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/Serialization.h"
@@ -158,6 +159,10 @@
init(""),
};
+list<std::string> DisabledClangTidyChecks{
+ "disable-clang-tidy-checks", cat(Features),
+ desc("List of clang-tidy checks to disable"), Hidden, CommaSeparated};
+
opt<CodeCompleteOptions::CodeCompletionParse> CodeCompletionParse{
"completion-parse",
cat(Features),
@@ -803,35 +808,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;
- 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;
- };
+ 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())
+ Providers.push_back(fixedTidyProvider(ClangTidyChecks));
+ else
+ Providers.push_back(provideDefaultChecks());
+ Providers.push_back(disableUnusableChecks(DisabledClangTidyChecks));
+ 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,59 @@
+//===--- 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"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+namespace detail {
+using TidyProviderInterface = void(tidy::ClangTidyOptions &,
+ /*Filename=*/llvm::StringRef,
+ /*Priority=*/unsigned &);
+} // namespace detail
+
+using TidyProvider = llvm::unique_function<detail::TidyProviderInterface>;
+using TidyProviderRef = llvm::function_ref<detail::TidyProviderInterface>;
+
+TidyProvider combine(std::vector<TidyProvider> Providers);
+std::unique_ptr<tidy::ClangTidyOptionsProvider>
+ asClangTidyOptionsProvider(TidyProviderRef);
+
+/// Provider that just sets the defaults.
+TidyProvider provideEnvironment();
+
+/// Provider that will enable a nice set of default checks if none are
+/// specified.
+TidyProvider provideDefaultChecks();
+
+/// Provider the enables a specific set of checks and warnings as errors.
+TidyProvider fixedTidyProvider(llvm::StringRef Checks,
+ llvm::StringRef WarningsAsErrors = {});
+
+/// Provider that will disable checks known to not work with clangd.
+TidyProvider
+disableUnusableChecks(std::vector<std::string> ChecksToDisable = {});
+
+/// Provider that searches for .clang-tidy configuration files in the directory
+/// tree.
+TidyProvider provideClangTidyFiles(ThreadsafeFS &);
+
+// Provider that uses clangd configuration files.
+TidyProvider provideClangdConfig();
+
+} // 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,219 @@
+//===--- 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 "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());
+
+ llvm::ErrorOr<llvm::vfs::Status> DirectoryStatus = FS->status(Directory);
+
+ if (!DirectoryStatus || !DirectoryStatus->isDirectory()) {
+ llvm::errs() << "Error reading configuration from " << Directory
+ << ": directory doesn't exist.\n";
+ return llvm::None;
+ }
+
+ 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()) {
+ llvm::errs() << "Can't read " << ConfigFile << ": " << EC.message() << "\n";
+ 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())
+ llvm::errs() << "Error parsing " << ConfigFile << ": "
+ << ParsedOptions.getError().message() << "\n";
+ return llvm::None;
+ }
+ return std::move(*ParsedOptions);
+}
+
+TidyProvider provideEnvironment() {
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) {
+ 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
+ Opts.mergeWith(EmptyDefaults, Order);
+ };
+}
+
+TidyProvider provideDefaultChecks() {
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) {
+ if (!Opts.Checks || Opts.Checks->empty()) {
+ // 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");
+ Opts.Checks->assign(DefaultChecks);
+ }
+ };
+}
+
+TidyProvider fixedTidyProvider(llvm::StringRef Checks,
+ llvm::StringRef WarningsAsErrors) {
+ return [Checks = std::string(Checks),
+ WarningsAsErrors = std::string(WarningsAsErrors)](
+ tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) {
+ mergeCheckList(Opts.Checks, Checks);
+ mergeCheckList(Opts.WarningsAsErrors, WarningsAsErrors);
+ };
+}
+
+TidyProvider disableUnusableChecks(std::vector<std::string> ChecksToDisable) {
+ for (std::string &Str : ChecksToDisable) {
+ if (!Str.empty() && Str.front() != '-')
+ Str.insert(Str.begin(), '-');
+ }
+ return [ChecksToDisable = std::move(ChecksToDisable)](
+ tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) {
+ if (Opts.Checks && !Opts.Checks->empty()) {
+ 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");
+ Opts.Checks =
+ llvm::join_items(",", *Opts.Checks, FalsePositives, CrashingChecks,
+ llvm::join(ChecksToDisable, ","));
+ }
+ };
+}
+
+TidyProvider provideClangdConfig() {
+ return [](tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &Order) {
+ 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(), Order));
+ };
+}
+
+TidyProvider provideClangTidyFiles(ThreadsafeFS &TFS) {
+ return [&](tidy::ClangTidyOptions &Opts, llvm::StringRef Filename,
+ unsigned &Order) {
+ llvm::SmallVector<tidy::ClangTidyOptions, 4> OptionStack;
+ auto FS(TFS.view(llvm::None));
+ llvm::SmallString<128> AbsolutePath(Filename);
+
+ if (FS->makeAbsolute(AbsolutePath))
+ return;
+
+ llvm::StringRef Path = llvm::sys::path::parent_path(AbsolutePath);
+ for (llvm::StringRef CurrentPath = Path; !CurrentPath.empty();
+ CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
+ auto ConfigFile = tryReadConfigFile(FS.get(), CurrentPath);
+ if (!ConfigFile)
+ continue;
+ OptionStack.push_back(std::move(*ConfigFile));
+ if (!OptionStack.back().InheritParentConfig.getValueOr(false))
+ break;
+ }
+ 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,
+ unsigned &Order) mutable {
+ for (auto &Provider : Providers) {
+ Provider(Opts, Filename, Order);
+ ++Order;
+ }
+ };
+}
+
+namespace {
+
+class TidyProviderInstance : public tidy::ClangTidyOptionsProvider {
+public:
+ TidyProviderInstance(TidyProviderRef Provider) : Provider(Provider) {}
+
+ const tidy::ClangTidyGlobalOptions &getGlobalOptions() override {
+ return Defaults;
+ }
+ std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override {
+ std::vector<OptionsSource> Opts;
+ Opts.emplace_back();
+ unsigned Priority = 0;
+ Provider(Opts.back().first, FileName, Priority);
+ return Opts;
+ }
+
+private:
+ TidyProviderRef Provider;
+ static const tidy::ClangTidyGlobalOptions Defaults;
+};
+const tidy::ClangTidyGlobalOptions TidyProviderInstance::Defaults;
+} // namespace
+
+std::unique_ptr<tidy::ClangTidyOptionsProvider>
+asClangTidyOptionsProvider(TidyProviderRef Provider) {
+ return std::make_unique<TidyProviderInstance>(Provider);
+}
+} // 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"
@@ -236,6 +237,10 @@
} // namespace
+/// Empty clang tidy provider, using this as a provider will disable clang-tidy.
+static void emptyTidyProvider(tidy::ClangTidyOptions &, llvm::StringRef,
+ unsigned &) {}
+
llvm::Optional<ParsedAST>
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
@@ -292,16 +297,17 @@
llvm::Optional<tidy::ClangTidyContext> CTContext;
{
trace::Span Tracer("ClangTidyInit");
- dlog("ClangTidy configuration for file {0}: {1}", Filename,
- tidy::configurationAsText(Inputs.Opts.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));
+ CTContext.emplace(asClangTidyOptionsProvider(Inputs.ClangTidyProvider
+ ? Inputs.ClangTidyProvider
+ : emptyTidyProvider));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(Filename);
+ dlog("ClangTidy configuration for file {0}: {1}", Filename,
+ tidy::configurationAsText(CTContext->getOptions()));
CTChecks = CTFactories.createChecks(CTContext.getPointer());
llvm::erase_if(CTChecks, [&](const auto &Check) {
return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
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.
@@ -377,7 +367,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
