njames93 created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74689
Files:
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
@@ -1,7 +1,12 @@
// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
// RUN: -format-style=llvm \
// RUN: -config='{CheckOptions: \
-// RUN: [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
+// RUN: [{key: performance-inefficient-vector-operation.EnableProto, value: 1}, \
+// RUN: {key: performance-inefficient-vector-operation.VectorLikeClasses, value : MyContainer}, \
+// RUN: {key: performance-inefficient-vector-operation.SupportedRanges, value : MyContainer}, \
+// RUN: {key: performance-inefficient-vector-operation.ReserveNames, value : Reserve}, \
+// RUN: {key: performance-inefficient-vector-operation.AppendNames, value : PushBack}, \
+// RUN: {key: performance-inefficient-vector-operation.SizeNames, value : Size}, ]}'
namespace std {
@@ -359,3 +364,160 @@
}
}
}
+
+namespace OptionsValidMatchDefault {
+template <typename T>
+class MyContainer {
+public:
+ unsigned size();
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+ void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+ MyContainer<int> CC1;
+ // CHECK-FIXES: {{^}} CC1.reserve(C.size());
+ for (auto I : C) {
+ CC1.push_back(I);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace OptionsValidMatchDefault
+
+namespace OptionsValidMatchDifferentMethods {
+template <typename T>
+class MyContainer {
+public:
+ unsigned Size();
+ T *begin() const;
+ T *end() const;
+ void PushBack(const T &);
+ void Reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+ MyContainer<int> CC2;
+ // CHECK-FIXES: {{^}} CC2.Reserve(C.Size());
+ for (auto I : C) {
+ CC2.PushBack(I);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'PushBack' is called
+ }
+}
+} // namespace OptionsValidMatchDifferentMethods
+
+namespace UnknownContainer {
+template <typename T>
+class MyUContainer {
+public:
+ unsigned size();
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+ void reserve(unsigned);
+};
+
+void foo(const MyUContainer<int> &C) {
+ // MyUContainer isn't specified as a VectorLikeClass in the Config Options
+ MyUContainer<int> CC3;
+ // CHECK-FIXES-NOT: {{^}} CC3.reserve(C.size());
+ for (auto I : C) {
+ CC3.push_back(I);
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace UnknownContainer
+
+namespace PrivateMethods {
+namespace Size {
+template <typename T>
+class MyContainer {
+ unsigned size();
+
+public:
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+ void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+ // MyContainer<int>::size is private, so calling it will be invalid
+ MyContainer<int> CC4;
+ // CHECK-FIXES-NOT: {{^}} CC4.reserve(C.size());
+ for (auto I : C) {
+ CC4.push_back(I);
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace Size
+namespace Reserve {
+template <typename T>
+class MyContainer {
+public:
+ unsigned size();
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+
+private:
+ void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+ // MyContainer<int>::reserve is private, so calling it will be invalid
+ MyContainer<int> CC5;
+ // CHECK-FIXES-NOT: {{^}} CC5.reserve(C.size());
+ for (auto I : C) {
+ CC5.push_back(I);
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace Reserve
+} // namespace PrivateMethods
+
+namespace BadSignatures {
+namespace Size {
+template <typename T>
+class MyContainer {
+public:
+ char *size();
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+ void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+ // MyContainer<int>::size doesn't return an integral type(char *), so ignore this class
+ MyContainer<int> CC6;
+ // CHECK-FIXES-NOT: {{^}} CC6.reserve(C.size());
+ for (auto I : C) {
+ CC6.push_back(I);
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace Size
+namespace Reserve {
+template <typename T>
+class MyContainer {
+public:
+ unsigned size();
+ T *begin() const;
+ T *end() const;
+ void push_back(const T &);
+ void reserve();
+};
+
+void foo(const MyContainer<int> &C) {
+ // MyContainer<int>::reserve doesn't take a single integral argument, so ignore this class
+ MyContainer<int> CC7;
+ // CHECK-FIXES-NOT: {{^}} CC7.reserve(C.size());
+ for (auto I : C) {
+ CC7.push_back(I);
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+ }
+}
+} // namespace Reserve
+} // namespace BadSignatures
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
@@ -34,9 +34,7 @@
}
* For-range loops like ``for (range-declaration : range_expression)``, the type
- of ``range_expression`` can be ``std::vector``, ``std::array``,
- ``std::deque``, ``std::set``, ``std::unordered_set``, ``std::map``,
- ``std::unordered_set``:
+ of ``range_expression`` are specified in :option:`SupportedRanges`:
.. code-block:: c++
@@ -59,6 +57,32 @@
Semicolon-separated list of names of vector-like classes. By default only
``::std::vector`` is considered.
+.. option:: AppendNames
+
+ Semicolon-separated list of names of append methods in :option:`VectorLikeClasses`.
+ By default only ``push_back`` and ``emplace_back`` are considered.
+
+.. option:: ReserveNames
+
+ Semicolon-separated list of names of reserve methods in :option:`VectorLikeClasses`.
+ By default only ``reserve`` is considered.
+
+ Note the method must have `1` parameter that is of integral type.
+
+.. option:: SupportedRanges
+
+ Semicolon-separated list of names of Range types for for-range expressions.
+ By default only ``std::vector``, ``std::array``, ``std::deque``,
+ ``std::set``, ``std::unordered_set``, ``std::map`` and ``std::unordered_set``
+ are considered.
+
+.. option:: SizeNames
+
+ Semicolon-separated list of names of size methods :option:`SupportedRanges`.
+ By default only ``size`` is considered.
+
+ Note the method must have `0` parameters and return an integral type.
+
.. option:: EnableProto
When non-zero, the check will also warn on inefficient operations for proto
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,10 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`performance-inefficient-vector-operation
+ <clang-tidy/checks/performance-inefficient-vector-operation>` check now has
+ extra options for custom containers.
+
- Improved :doc:`readability-qualified-auto
<clang-tidy/checks/readability-qualified-auto>` check now supports a
`AddConstToQualified` to enable adding ``const`` qualifiers to variables
Index: clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
+++ clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
@@ -35,7 +35,11 @@
StringRef VarDeclName, StringRef VarDeclStmtName,
const ast_matchers::DeclarationMatcher &AppendMethodDecl,
StringRef AppendCallName, ast_matchers::MatchFinder *Finder);
- const std::vector<std::string> VectorLikeClasses;
+ const std::string VectorLikeClasses;
+ const std::string AppendNames;
+ const std::string ReserveNames;
+ const std::string SupportedRanges;
+ const std::string SizeNames;
// If true, also check inefficient operations for proto repeated fields.
bool EnableProto;
Index: clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -7,11 +7,12 @@
//===----------------------------------------------------------------------===//
#include "InefficientVectorOperationCheck.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
-#include "../utils/DeclRefExprUtils.h"
-#include "../utils/OptionsUtils.h"
using namespace clang::ast_matchers;
@@ -54,6 +55,7 @@
static const char LoopParentName[] = "loop_parent";
static const char VectorVarDeclName[] = "vector_var_decl";
static const char VectorVarDeclStmtName[] = "vector_var_decl_stmt";
+static const char VectorReserveName[] = "vector_reserve_name";
static const char PushBackOrEmplaceBackCallName[] = "append_call";
static const char ProtoVarDeclName[] = "proto_var_decl";
static const char ProtoVarDeclStmtName[] = "proto_var_decl_stmt";
@@ -61,11 +63,32 @@
static const char LoopInitVarName[] = "loop_init_var";
static const char LoopEndExprName[] = "loop_end_expr";
static const char RangeLoopName[] = "for_range_loop";
+static const char RangeLoopSizeMethod[] = "for_range_size_name";
+static const char SupportedDefaultRanges[] = "::std::vector;"
+ "::std::set;"
+ "::std::unordered_set;"
+ "::std::map;"
+ "::std::unordered_map;"
+ "::std::array;"
+ "::std::deque";
-ast_matchers::internal::Matcher<Expr> supportedContainerTypesMatcher() {
- return hasType(cxxRecordDecl(hasAnyName(
- "::std::vector", "::std::set", "::std::unordered_set", "::std::map",
- "::std::unordered_map", "::std::array", "::std::deque")));
+ast_matchers::internal::Matcher<NamedDecl>
+hasAnyNameStdString(std::vector<std::string> Names) {
+ return ast_matchers::internal::Matcher<NamedDecl>(
+ new ast_matchers::internal::HasNameMatcher(std::move(Names)));
+}
+
+static std::vector<std::string> parseStringListPair(StringRef LHS,
+ StringRef RHS) {
+ if (LHS.empty()) {
+ if (RHS.empty())
+ return {};
+ return utils::options::parseStringList(RHS);
+ }
+ if (RHS.empty())
+ return utils::options::parseStringList(LHS);
+ llvm::SmallString<512> Buffer;
+ return utils::options::parseStringList((LHS + ";" + RHS).toStringRef(Buffer));
}
} // namespace
@@ -73,14 +96,20 @@
InefficientVectorOperationCheck::InefficientVectorOperationCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- VectorLikeClasses(utils::options::parseStringList(
- Options.get("VectorLikeClasses", "::std::vector"))),
+ VectorLikeClasses(Options.get("VectorLikeClasses", "")),
+ AppendNames(Options.get("AppendNames", "")),
+ ReserveNames(Options.get("ReserveNames", "")),
+ SupportedRanges(Options.get("SupportedRanges", "")),
+ SizeNames(Options.get("SizeNames", "")),
EnableProto(Options.getLocalOrGlobal("EnableProto", false)) {}
void InefficientVectorOperationCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, "VectorLikeClasses",
- utils::options::serializeStringList(VectorLikeClasses));
+ Options.store(Opts, "VectorLikeClasses", VectorLikeClasses);
+ Options.store(Opts, "AppendNames", AppendNames);
+ Options.store(Opts, "ReserveNames", ReserveNames);
+ Options.store(Opts, "SupportedRanges", SupportedRanges);
+ Options.store(Opts, "SizeNames", SizeNames);
Options.store(Opts, "EnableProto", EnableProto);
}
@@ -145,17 +174,30 @@
// FIXME: Support more complex range-expressions.
Finder->addMatcher(
cxxForRangeStmt(
- hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
+ hasRangeInit(declRefExpr(hasType(cxxRecordDecl(
+ hasAnyNameStdString(
+ parseStringListPair(SupportedDefaultRanges, SupportedRanges)),
+ hasMethod(cxxMethodDecl(hasAnyNameStdString(parseStringListPair(
+ "size", SizeNames)),
+ parameterCountIs(0), isPublic(),
+ returns(isInteger()))
+ .bind(RangeLoopSizeMethod)))))),
HasInterestingLoopBody, InInterestingCompoundStmt)
.bind(RangeLoopName),
this);
}
void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
- const auto VectorDecl = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
- VectorLikeClasses.begin(), VectorLikeClasses.end())));
- const auto AppendMethodDecl =
- cxxMethodDecl(hasAnyName("push_back", "emplace_back"));
+ const auto VectorDecl = cxxRecordDecl(
+ hasAnyNameStdString(
+ parseStringListPair("::std::vector", VectorLikeClasses)),
+ hasMethod(cxxMethodDecl(hasAnyNameStdString(
+ parseStringListPair("reserve", ReserveNames)),
+ isPublic(), parameterCountIs(1),
+ hasParameter(0, hasType(isInteger())))
+ .bind(VectorReserveName)));
+ const auto AppendMethodDecl = cxxMethodDecl(hasAnyNameStdString(
+ parseStringListPair("push_back;emplace_back", AppendNames)));
AddMatcher(VectorDecl, VectorVarDeclName, VectorVarDeclStmtName,
AppendMethodDecl, PushBackOrEmplaceBackCallName, Finder);
@@ -224,7 +266,10 @@
std::string PartialReserveStmt;
if (VectorAppendCall != nullptr) {
- PartialReserveStmt = ".reserve";
+ const auto *ReserveMethod =
+ Result.Nodes.getNodeAs<CXXMethodDecl>(VectorReserveName);
+ assert(ReserveMethod && "Reserve Method should not be null");
+ PartialReserveStmt = ("." + ReserveMethod->getName()).str();
} else {
llvm::StringRef FieldName = ProtoAddFieldCall->getMethodDecl()->getName();
FieldName.consume_front("add_");
@@ -247,7 +292,11 @@
Lexer::getSourceText(CharSourceRange::getTokenRange(
RangeLoop->getRangeInit()->getSourceRange()),
SM, Context->getLangOpts());
- ReserveSize = (RangeInitExpName + ".size()").str();
+ const auto *SizeMethod =
+ Result.Nodes.getNodeAs<CXXMethodDecl>(RangeLoopSizeMethod);
+ assert(SizeMethod &&
+ "SizeMethod should be valid in a matched RangeForLoop");
+ ReserveSize = (RangeInitExpName + "." + SizeMethod->getName() + "()").str();
} else if (ForLoop) {
// Handle counter-based loop cases.
StringRef LoopEndSource = Lexer::getSourceText(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
