llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) <details> <summary>Changes</summary> `GlobList` is a polymorphic class, but we don't use it polymorphically anywhere, so we seem to be paying that cost unnecessarily. --- Full diff: https://github.com/llvm/llvm-project/pull/164212.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp (+4-7) - (modified) clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h (+3-3) - (modified) clang-tools-extra/clang-tidy/GlobList.h (+3-5) - (modified) clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp (+7-9) ``````````diff diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 65fd09f99ef0f..d036202bf932f 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -238,9 +238,8 @@ static bool parseFileExtensions(llvm::ArrayRef<std::string> AllFileExtensions, void ClangTidyContext::setCurrentFile(StringRef File) { CurrentFile = std::string(File); CurrentOptions = getOptionsForFile(CurrentFile); - CheckFilter = std::make_unique<CachedGlobList>(*getOptions().Checks); - WarningAsErrorFilter = - std::make_unique<CachedGlobList>(*getOptions().WarningsAsErrors); + CheckFilter = {*getOptions().Checks}; + WarningAsErrorFilter = {*getOptions().WarningsAsErrors}; if (!parseFileExtensions(*getOptions().HeaderFileExtensions, HeaderFileExtensions)) this->configurationDiag("Invalid header file extensions"); @@ -284,13 +283,11 @@ ClangTidyContext::getProfileStorageParams() const { } bool ClangTidyContext::isCheckEnabled(StringRef CheckName) const { - assert(CheckFilter != nullptr); - return CheckFilter->contains(CheckName); + return CheckFilter.contains(CheckName); } bool ClangTidyContext::treatAsError(StringRef CheckName) const { - assert(WarningAsErrorFilter != nullptr); - return WarningAsErrorFilter->contains(CheckName); + return WarningAsErrorFilter.contains(CheckName); } std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const { diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 21ffd9de35c19..ddf2c378b9ff0 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -12,6 +12,7 @@ #include "ClangTidyOptions.h" #include "ClangTidyProfiling.h" #include "FileExtensionsSet.h" +#include "GlobList.h" #include "NoLintDirectiveHandler.h" #include "clang/Basic/Diagnostic.h" #include "clang/Tooling/Core/Diagnostic.h" @@ -27,7 +28,6 @@ class ASTContext; class SourceManager; namespace tidy { -class CachedGlobList; /// A detected error complete with information to display diagnostic and /// automatic fix. @@ -247,8 +247,8 @@ class ClangTidyContext { std::string CurrentFile; ClangTidyOptions CurrentOptions; - std::unique_ptr<CachedGlobList> CheckFilter; - std::unique_ptr<CachedGlobList> WarningAsErrorFilter; + CachedGlobList CheckFilter; + CachedGlobList WarningAsErrorFilter; FileExtensionsSet HeaderFileExtensions; FileExtensionsSet ImplementationFileExtensions; diff --git a/clang-tools-extra/clang-tidy/GlobList.h b/clang-tools-extra/clang-tidy/GlobList.h index c9086df2b7973..4d6f0ede717a1 100644 --- a/clang-tools-extra/clang-tidy/GlobList.h +++ b/clang-tools-extra/clang-tidy/GlobList.h @@ -24,8 +24,6 @@ namespace clang::tidy { /// them in the order of appearance in the list. class GlobList { public: - virtual ~GlobList() = default; - /// \p Globs is a comma-separated list of globs (only the '*' metacharacter is /// supported) with an optional '-' prefix to denote exclusion. /// @@ -38,7 +36,7 @@ class GlobList { /// Returns \c true if the pattern matches \p S. The result is the last /// matching glob's Positive flag. - virtual bool contains(StringRef S) const; + bool contains(StringRef S) const; private: struct GlobListItem { @@ -54,12 +52,12 @@ class GlobList { /// A \p GlobList that caches search results, so that search is performed only /// once for the same query. -class CachedGlobList final : public GlobList { +class CachedGlobList : public GlobList { public: using GlobList::GlobList; /// \see GlobList::contains - bool contains(StringRef S) const override; + bool contains(StringRef S) const; private: mutable llvm::StringMap<bool> Cache; diff --git a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp index ef20ee18347df..bcaf38c6aa4dd 100644 --- a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp +++ b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp @@ -79,9 +79,8 @@ class NoLintToken { // - Negative globs ignored (which would effectively disable the suppression). NoLintToken(NoLintType Type, size_t Pos, const std::optional<StringRef> &Checks) - : Type(Type), Pos(Pos), ChecksGlob(std::make_unique<CachedGlobList>( - Checks.value_or("*"), - /*KeepNegativeGlobs=*/false)) { + : Type(Type), Pos(Pos), ChecksGlob(Checks.value_or("*"), + /*KeepNegativeGlobs=*/false) { if (Checks) this->Checks = trimWhitespace(*Checks); } @@ -93,13 +92,13 @@ class NoLintToken { size_t Pos; // A glob of the checks this NOLINT token disables. - std::unique_ptr<CachedGlobList> ChecksGlob; + CachedGlobList ChecksGlob; // If this NOLINT specifies checks, return the checks. const std::optional<std::string> &checks() const { return Checks; } // Whether this NOLINT applies to the provided check. - bool suppresses(StringRef Check) const { return ChecksGlob->contains(Check); } + bool suppresses(StringRef Check) const { return ChecksGlob.contains(Check); } private: std::optional<std::string> Checks; @@ -156,21 +155,20 @@ namespace { // Represents a source range within a pair of NOLINT(BEGIN/END) comments. class NoLintBlockToken { public: - NoLintBlockToken(size_t BeginPos, size_t EndPos, - std::unique_ptr<CachedGlobList> ChecksGlob) + NoLintBlockToken(size_t BeginPos, size_t EndPos, CachedGlobList ChecksGlob) : BeginPos(BeginPos), EndPos(EndPos), ChecksGlob(std::move(ChecksGlob)) {} // Whether the provided diagnostic is within and is suppressible by this block // of NOLINT(BEGIN/END) comments. bool suppresses(size_t DiagPos, StringRef DiagName) const { return (BeginPos < DiagPos) && (DiagPos < EndPos) && - ChecksGlob->contains(DiagName); + ChecksGlob.contains(DiagName); } private: size_t BeginPos; size_t EndPos; - std::unique_ptr<CachedGlobList> ChecksGlob; + CachedGlobList ChecksGlob; }; } // namespace `````````` </details> https://github.com/llvm/llvm-project/pull/164212 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
