kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Depends on D81998 <https://reviews.llvm.org/D81998>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82002
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -15,6 +15,7 @@
#include "index/Background.h"
#include "index/Serialization.h"
#include "refactor/Rename.h"
+#include "support/FSProvider.h"
#include "support/Path.h"
#include "support/Shutdown.h"
#include "support/Trace.h"
@@ -472,6 +473,51 @@
const char TestScheme::TestDir[] = "/clangd-test";
#endif
+// A thread-safe options provider suitable for use by ClangdServer. It also
+// provides some default checks if user has specified none.
+class ClangdTidyOptionsProvider : public clang::tidy::FileOptionsProvider {
+public:
+ ClangdTidyOptionsProvider(const tidy::ClangTidyOptions &OverrideOptions,
+ const ThreadSafeFS &FSProvider)
+ : FileOptionsProvider(tidy::ClangTidyGlobalOptions(),
+ tidy::ClangTidyOptions(), OverrideOptions,
+ FSProvider.view(llvm::None)) {}
+
+ std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override {
+ std::vector<OptionsSource> Sources;
+ {
+ std::lock_guard<std::mutex> Lock(Mu);
+ Sources = tidy::FileOptionsProvider::getRawOptions(FileName);
+ }
+ // If the user hasn't configured clang-tidy checks at all, including via
+ // .clang-tidy, give them a nice set of checks.
+ // (This should be what the "default" options does, but it isn't...)
+ bool HasChecks = false;
+ for (const auto &Source : Sources)
+ HasChecks |= Source.first.Checks.hasValue();
+ if (!HasChecks)
+ Sources.push_back(DefaultClangdSource);
+ return Sources;
+ }
+
+private:
+ std::mutex Mu;
+ const OptionsSource DefaultClangdSource = []() -> OptionsSource {
+ tidy::ClangTidyOptions Opts;
+ // These default checks are chosen for:
+ // - low false-positive rate
+ // - providing a lot of value
+ // - being reasonably efficient
+ Opts.Checks = 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 {Opts, "-clangd-opts"};
+ }();
+};
} // namespace
} // namespace clangd
} // namespace clang
@@ -705,49 +751,15 @@
TransportLayer = createPathMappingTransport(std::move(TransportLayer),
std::move(*Mappings));
}
- // Create an empty clang-tidy option.
- std::mutex ClangTidyOptMu;
- std::unique_ptr<tidy::ClangTidyOptionsProvider>
- ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/
+ std::unique_ptr<tidy::ClangTidyOptionsProvider> ClangTidyOptProvider;
if (EnableClangTidy) {
- auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults();
- EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
tidy::ClangTidyOptions OverrideClangTidyOptions;
if (!ClangTidyChecks.empty())
OverrideClangTidyOptions.Checks = ClangTidyChecks;
- ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
- tidy::ClangTidyGlobalOptions(),
- /* Default */ EmptyDefaults,
- /* Override */ OverrideClangTidyOptions, FSProvider.view(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);
- }
- if (!Opts.Checks) {
- // If the user hasn't configured clang-tidy checks at all, including
- // via .clang-tidy, give them a nice set of checks.
- // (This should be what the "default" options does, but it isn't...)
- //
- // These default checks are chosen for:
- // - low false-positive rate
- // - providing a lot of value
- // - being reasonably efficient
- Opts.Checks = 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 Opts;
- };
+ ClangTidyOptProvider = std::make_unique<ClangdTidyOptionsProvider>(
+ OverrideClangTidyOptions, FSProvider);
}
+ Opts.TidyOptProvider = ClangTidyOptProvider.get();
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -40,12 +40,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.
///
@@ -118,7 +112,8 @@
/// Clangd supports only a small subset of ClangTidyOptions, these options
/// (Checks, CheckOptions) are about which clang-tidy checks will be
/// enabled.
- ClangTidyOptionsBuilder GetClangTidyOptions;
+ /// Note that this must be thread-safe.
+ tidy::ClangTidyOptionsProvider *TidyOptProvider = nullptr;
/// If true, turn on the `-frecovery-ast` clang flag.
bool BuildRecoveryAST = true;
@@ -347,7 +342,7 @@
std::vector<std::unique_ptr<SymbolIndex>> MergedIdx;
// When set, provides clang-tidy options for a specific file.
- ClangTidyOptionsBuilder GetClangTidyOptions;
+ tidy::ClangTidyOptionsProvider *TidyOptProvider = nullptr;
// 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
@@ -135,7 +135,7 @@
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
: nullptr),
- GetClangTidyOptions(Opts.GetClangTidyOptions),
+ TidyOptProvider(Opts.TidyOptProvider),
SuggestMissingIncludes(Opts.SuggestMissingIncludes),
BuildRecoveryAST(Opts.BuildRecoveryAST),
PreserveRecoveryASTType(Opts.PreserveRecoveryASTType),
@@ -182,9 +182,8 @@
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(*FSProvider.view(llvm::None), File);
+ if (TidyOptProvider)
+ Opts.ClangTidyOpts = TidyOptProvider->getOptions(File);
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
// Compile command is set asynchronously during update, as it can be slow.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
