diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/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; - /// 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> 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). diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/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. diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/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,57 @@ 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::ClangTidyOptionsProvider { +public: + ClangdTidyOptionsProvider( + std::unique_ptr Inner) + : Inner(std::move(Inner)) { + assert(this->Inner); + } + + std::vector getRawOptions(llvm::StringRef FileName) override { + std::vector Sources; + { + std::lock_guard Lock(Mu); + Sources = Inner->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; + } + + const tidy::ClangTidyGlobalOptions &getGlobalOptions() override { + std::lock_guard Lock(Mu); + return Inner->getGlobalOptions(); + } + +private: + std::mutex Mu; + std::unique_ptr Inner; + 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 +757,20 @@ TransportLayer = createPathMappingTransport(std::move(TransportLayer), std::move(*Mappings)); } - // Create an empty clang-tidy option. - std::mutex ClangTidyOptMu; - std::unique_ptr - ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/ + std::unique_ptr 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::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 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; - }; + auto InnerProvider = std::make_unique( + tidy::ClangTidyGlobalOptions(), std::move(EmptyDefaults), + std::move(OverrideClangTidyOptions), FSProvider.view(llvm::None)); + ClangTidyOptProvider = + std::make_unique(std::move(InnerProvider)); } + Opts.TidyOptProvider = ClangTidyOptProvider.get(); Opts.SuggestMissingIncludes = SuggestMissingIncludes; Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);