[clang-tools-extra] 62a090f - [clangd] Initialize clang-tidy modules only once
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed May 10 03:39:19 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-05-10T12:32:33+02:00
New Revision: 62a090f958ce02a8035e3c9424a05dbfe25859e1
URL: https://github.com/llvm/llvm-project/commit/62a090f958ce02a8035e3c9424a05dbfe25859e1
DIFF: https://github.com/llvm/llvm-project/commit/62a090f958ce02a8035e3c9424a05dbfe25859e1.diff
LOG: [clangd] Initialize clang-tidy modules only once
This is showing up on our profiles with ~100ms contribution @95th% for
buildAST latencies.
The patch is unlikely to address it all, but should help with some low-hanging
fruit.
Differential Revision: https://reviews.llvm.org/D150257
Added:
clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
Modified:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 1501a5c5f3c3b..e5a391a775389 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -9,6 +9,7 @@
#include "ParsedAST.h"
#include "../clang-tidy/ClangTidyCheck.h"
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModule.h"
#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
#include "Compiler.h"
@@ -25,7 +26,6 @@
#include "TidyProvider.h"
#include "clang-include-cleaner/Record.h"
#include "index/CanonicalIncludes.h"
-#include "index/Index.h"
#include "index/Symbol.h"
#include "support/Logger.h"
#include "support/Trace.h"
@@ -50,7 +50,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
-#include <algorithm>
#include <memory>
#include <optional>
#include <vector>
@@ -476,16 +475,19 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// diagnostics.
if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
- tidy::ClangTidyCheckFactories CTFactories;
- for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
- E.instantiate()->addCheckFactories(CTFactories);
+ static const auto *CTFactories = [] {
+ auto *CTFactories = new tidy::ClangTidyCheckFactories;
+ for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+ E.instantiate()->addCheckFactories(*CTFactories);
+ return CTFactories;
+ }();
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(Filename);
CTContext->setSelfContainedDiags(true);
- CTChecks = CTFactories.createChecksForLanguage(&*CTContext);
+ CTChecks = CTFactories->createChecksForLanguage(&*CTContext);
Preprocessor *PP = &Clang->getPreprocessor();
for (const auto &Check : CTChecks) {
Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
@@ -610,10 +612,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
Macros = Patch->mainFileMacros();
Marks = Patch->marks();
}
- auto& PP = Clang->getPreprocessor();
- PP.addPPCallbacks(
- std::make_unique<CollectMainFileMacros>(
- PP, Macros));
+ auto &PP = Clang->getPreprocessor();
+ PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(PP, Macros));
PP.addPPCallbacks(
collectPragmaMarksCallback(Clang->getSourceManager(), Marks));
diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp
index f3ed6f08a9acb..e3a6d5af20ae2 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -8,6 +8,7 @@
#include "TidyProvider.h"
#include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/ClangTidyOptions.h"
#include "Config.h"
#include "support/FileCache.h"
#include "support/Logger.h"
@@ -283,8 +284,15 @@ TidyProvider combine(std::vector<TidyProvider> Providers) {
tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
llvm::StringRef Filename) {
- tidy::ClangTidyOptions Opts = tidy::ClangTidyOptions::getDefaults();
- Opts.Checks->clear();
+ // getDefaults instantiates all check factories, which are registered at link
+ // time. So cache the results once.
+ static const auto *DefaultOpts = [] {
+ auto *Opts = new tidy::ClangTidyOptions;
+ *Opts = tidy::ClangTidyOptions::getDefaults();
+ Opts->Checks->clear();
+ return Opts;
+ }();
+ auto Opts = *DefaultOpts;
if (Provider)
Provider(Opts, Filename);
return Opts;
diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index 6d7bb59be3ab1..8d02b91fdd716 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -79,8 +79,9 @@ add_unittest(ClangdUnitTests ClangdTests
PrintASTTests.cpp
ProjectAwareIndexTests.cpp
QualityTests.cpp
- RenameTests.cpp
RIFFTests.cpp
+ RenameTests.cpp
+ ReplayPeambleTests.cpp
SelectionTests.cpp
SemanticHighlightingTests.cpp
SemanticSelectionTests.cpp
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 5792359e3c785..8d5d0630de165 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -12,8 +12,6 @@
//===----------------------------------------------------------------------===//
#include "../../clang-tidy/ClangTidyCheck.h"
-#include "../../clang-tidy/ClangTidyModule.h"
-#include "../../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
#include "Annotations.h"
#include "Compiler.h"
@@ -29,8 +27,6 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
-#include "clang/Lex/PPCallbacks.h"
-#include "clang/Lex/Token.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Testing/Support/Error.h"
@@ -96,10 +92,6 @@ MATCHER_P(withTemplateArgs, ArgName, "") {
return false;
}
-MATCHER_P(rangeIs, R, "") {
- return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
-}
-
MATCHER_P(pragmaTrivia, P, "") { return arg.Trivia == P; }
MATCHER(eqInc, "") {
@@ -352,123 +344,6 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
MATCHER_P(withFileName, Inc, "") { return arg.FileName == Inc; }
-TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
- struct Inclusion {
- Inclusion(const SourceManager &SM, SourceLocation HashLoc,
- const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled,
- CharSourceRange FilenameRange)
- : HashOffset(SM.getDecomposedLoc(HashLoc).second), IncTok(IncludeTok),
- IncDirective(IncludeTok.getIdentifierInfo()->getName()),
- FileNameOffset(SM.getDecomposedLoc(FilenameRange.getBegin()).second),
- FileName(FileName), IsAngled(IsAngled) {
- EXPECT_EQ(
- toSourceCode(SM, FilenameRange.getAsRange()).drop_back().drop_front(),
- FileName);
- }
- size_t HashOffset;
- syntax::Token IncTok;
- llvm::StringRef IncDirective;
- size_t FileNameOffset;
- llvm::StringRef FileName;
- bool IsAngled;
- };
- static std::vector<Inclusion> Includes;
- static std::vector<syntax::Token> SkippedFiles;
- struct ReplayPreamblePPCallback : public PPCallbacks {
- const SourceManager &SM;
- explicit ReplayPreamblePPCallback(const SourceManager &SM) : SM(SM) {}
-
- void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
- StringRef FileName, bool IsAngled,
- CharSourceRange FilenameRange, OptionalFileEntryRef,
- StringRef, StringRef, const clang::Module *,
- SrcMgr::CharacteristicKind) override {
- Includes.emplace_back(SM, HashLoc, IncludeTok, FileName, IsAngled,
- FilenameRange);
- }
-
- void FileSkipped(const FileEntryRef &, const Token &FilenameTok,
- SrcMgr::CharacteristicKind) override {
- SkippedFiles.emplace_back(FilenameTok);
- }
- };
- struct ReplayPreambleCheck : public tidy::ClangTidyCheck {
- ReplayPreambleCheck(StringRef Name, tidy::ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
- void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
- Preprocessor *ModuleExpanderPP) override {
- PP->addPPCallbacks(::std::make_unique<ReplayPreamblePPCallback>(SM));
- }
- };
- struct ReplayPreambleModule : public tidy::ClangTidyModule {
- void
- addCheckFactories(tidy::ClangTidyCheckFactories &CheckFactories) override {
- CheckFactories.registerCheck<ReplayPreambleCheck>(
- "replay-preamble-check");
- }
- };
-
- static tidy::ClangTidyModuleRegistry::Add<ReplayPreambleModule> X(
- "replay-preamble-module", "");
- TestTU TU;
- // This check records inclusion directives replayed by clangd.
- 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]]"
- $hash^#$include[[include]] $filebegin^<$filerange[[a.h]]>)cpp");
- llvm::StringRef Code = Test.code();
- TU.Code = Code.str();
- TU.AdditionalFiles["bar.h"] = "";
- TU.AdditionalFiles["baz.h"] = "";
- TU.AdditionalFiles["a.h"] = "";
- // Since we are also testing #import directives, and they don't make much
- // sense in c++ (also they actually break on windows), just set language to
- // obj-c.
- TU.ExtraArgs = {"-isystem.", "-xobjective-c"};
-
- const auto &AST = TU.build();
- const auto &SM = AST.getSourceManager();
-
- auto HashLocs = Test.points("hash");
- ASSERT_EQ(HashLocs.size(), Includes.size());
- auto IncludeRanges = Test.ranges("include");
- ASSERT_EQ(IncludeRanges.size(), Includes.size());
- auto FileBeginLocs = Test.points("filebegin");
- ASSERT_EQ(FileBeginLocs.size(), Includes.size());
- auto FileRanges = Test.ranges("filerange");
- ASSERT_EQ(FileRanges.size(), Includes.size());
-
- ASSERT_EQ(SkippedFiles.size(), Includes.size());
- for (size_t I = 0; I < Includes.size(); ++I) {
- const auto &Inc = Includes[I];
-
- EXPECT_EQ(Inc.HashOffset, HashLocs[I]);
-
- auto IncRange = IncludeRanges[I];
- EXPECT_THAT(Inc.IncTok.range(SM), rangeIs(IncRange));
- EXPECT_EQ(Inc.IncTok.kind(), tok::identifier);
- EXPECT_EQ(Inc.IncDirective,
- Code.substr(IncRange.Begin, IncRange.End - IncRange.Begin));
-
- EXPECT_EQ(Inc.FileNameOffset, FileBeginLocs[I]);
- EXPECT_EQ(Inc.IsAngled, Code[FileBeginLocs[I]] == '<');
-
- auto FileRange = FileRanges[I];
- EXPECT_EQ(Inc.FileName,
- Code.substr(FileRange.Begin, FileRange.End - FileRange.Begin));
-
- EXPECT_EQ(SM.getDecomposedLoc(SkippedFiles[I].location()).second,
- Inc.FileNameOffset);
- // This also contains quotes/angles so increment the range by one from both
- // sides.
- EXPECT_EQ(
- SkippedFiles[I].text(SM),
- Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
- EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
- }
-}
-
TEST(ParsedASTTest, PatchesAdditionalIncludes) {
llvm::StringLiteral ModifiedContents = R"cpp(
#include "baz.h"
diff --git a/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp b/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
new file mode 100644
index 0000000000000..459a8f4c36a4c
--- /dev/null
+++ b/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
@@ -0,0 +1,166 @@
+//===-- ReplayPreambleTests.cpp -------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// These tests cover clangd's logic to replay PP events from preamble to
+// clang-tidy checks.
+//
+//===----------------------------------------------------------------------===//
+
+#include "../../clang-tidy/ClangTidyCheck.h"
+#include "../../clang-tidy/ClangTidyModule.h"
+#include "../../clang-tidy/ClangTidyModuleRegistry.h"
+#include "AST.h"
+#include "Diagnostics.h"
+#include "ParsedAST.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TidyProvider.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/Module.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+#include "llvm/Testing/Annotations/Annotations.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <cstddef>
+#include <memory>
+#include <vector>
+
+namespace clang::clangd {
+namespace {
+struct Inclusion {
+ Inclusion(const SourceManager &SM, SourceLocation HashLoc,
+ const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled,
+ CharSourceRange FilenameRange)
+ : HashOffset(SM.getDecomposedLoc(HashLoc).second), IncTok(IncludeTok),
+ IncDirective(IncludeTok.getIdentifierInfo()->getName()),
+ FileNameOffset(SM.getDecomposedLoc(FilenameRange.getBegin()).second),
+ FileName(FileName), IsAngled(IsAngled) {
+ EXPECT_EQ(
+ toSourceCode(SM, FilenameRange.getAsRange()).drop_back().drop_front(),
+ FileName);
+ }
+ size_t HashOffset;
+ syntax::Token IncTok;
+ llvm::StringRef IncDirective;
+ size_t FileNameOffset;
+ llvm::StringRef FileName;
+ bool IsAngled;
+};
+static std::vector<Inclusion> Includes;
+static std::vector<syntax::Token> SkippedFiles;
+struct ReplayPreamblePPCallback : public PPCallbacks {
+ const SourceManager &SM;
+ explicit ReplayPreamblePPCallback(const SourceManager &SM) : SM(SM) {}
+
+ void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+ StringRef FileName, bool IsAngled,
+ CharSourceRange FilenameRange, OptionalFileEntryRef,
+ StringRef, StringRef, const clang::Module *,
+ SrcMgr::CharacteristicKind) override {
+ Includes.emplace_back(SM, HashLoc, IncludeTok, FileName, IsAngled,
+ FilenameRange);
+ }
+
+ void FileSkipped(const FileEntryRef &, const Token &FilenameTok,
+ SrcMgr::CharacteristicKind) override {
+ SkippedFiles.emplace_back(FilenameTok);
+ }
+};
+struct ReplayPreambleCheck : public tidy::ClangTidyCheck {
+ ReplayPreambleCheck(StringRef Name, tidy::ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override {
+ PP->addPPCallbacks(::std::make_unique<ReplayPreamblePPCallback>(SM));
+ }
+};
+llvm::StringLiteral CheckName = "replay-preamble-check";
+struct ReplayPreambleModule : public tidy::ClangTidyModule {
+ void
+ addCheckFactories(tidy::ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<ReplayPreambleCheck>(CheckName);
+ }
+};
+static tidy::ClangTidyModuleRegistry::Add<ReplayPreambleModule>
+ X("replay-preamble-module", "");
+
+MATCHER_P(rangeIs, R, "") {
+ return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
+}
+
+TEST(ReplayPreambleTest, IncludesAndSkippedFiles) {
+ TestTU TU;
+ // This check records inclusion directives replayed by clangd.
+ TU.ClangTidyProvider = addTidyChecks(CheckName);
+ llvm::Annotations Test(R"cpp(
+ $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
+ $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
+ $hash^#$include[[include]] $filebegin^<$filerange[[a.h]]>)cpp");
+ llvm::StringRef Code = Test.code();
+ TU.Code = Code.str();
+ TU.AdditionalFiles["bar.h"] = "";
+ TU.AdditionalFiles["baz.h"] = "";
+ TU.AdditionalFiles["a.h"] = "";
+ // Since we are also testing #import directives, and they don't make much
+ // sense in c++ (also they actually break on windows), just set language to
+ // obj-c.
+ TU.ExtraArgs = {"-isystem.", "-xobjective-c"};
+
+ const auto &AST = TU.build();
+ const auto &SM = AST.getSourceManager();
+
+ auto HashLocs = Test.points("hash");
+ ASSERT_EQ(HashLocs.size(), Includes.size());
+ auto IncludeRanges = Test.ranges("include");
+ ASSERT_EQ(IncludeRanges.size(), Includes.size());
+ auto FileBeginLocs = Test.points("filebegin");
+ ASSERT_EQ(FileBeginLocs.size(), Includes.size());
+ auto FileRanges = Test.ranges("filerange");
+ ASSERT_EQ(FileRanges.size(), Includes.size());
+
+ ASSERT_EQ(SkippedFiles.size(), Includes.size());
+ for (size_t I = 0; I < Includes.size(); ++I) {
+ const auto &Inc = Includes[I];
+
+ EXPECT_EQ(Inc.HashOffset, HashLocs[I]);
+
+ auto IncRange = IncludeRanges[I];
+ EXPECT_THAT(Inc.IncTok.range(SM), rangeIs(IncRange));
+ EXPECT_EQ(Inc.IncTok.kind(), tok::identifier);
+ EXPECT_EQ(Inc.IncDirective,
+ Code.substr(IncRange.Begin, IncRange.End - IncRange.Begin));
+
+ EXPECT_EQ(Inc.FileNameOffset, FileBeginLocs[I]);
+ EXPECT_EQ(Inc.IsAngled, Code[FileBeginLocs[I]] == '<');
+
+ auto FileRange = FileRanges[I];
+ EXPECT_EQ(Inc.FileName,
+ Code.substr(FileRange.Begin, FileRange.End - FileRange.Begin));
+
+ EXPECT_EQ(SM.getDecomposedLoc(SkippedFiles[I].location()).second,
+ Inc.FileNameOffset);
+ // This also contains quotes/angles so increment the range by one from both
+ // sides.
+ EXPECT_EQ(
+ SkippedFiles[I].text(SM),
+ Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
+ EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
+ }
+}
+} // namespace
+} // namespace clang::clangd
More information about the cfe-commits
mailing list