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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits