kuhar created this revision.
kuhar added a project: clang-tools-extra.
This patch prevents modernize-use-emplace from changing push_backs of brace
initialized pairs (`.push_back({1, 2})`) to `.emplace_back(1, 2)`.
Pair's constructor doesn't have any interesting logic and basically performs
aggregate initialization. There also doesn't seem to be much win
in terms of making code more concise, thus is makes little sense to turn such
push_back calls into emplace_backs.
The change is inspired by the recent discussion on changing and enforcing
coding guidelines:
[llvm-dev] [cfe-dev] Modernizing LLVM Coding Style Guide and enforcing
Clang-tidy
http://lists.llvm.org/pipermail/llvm-dev/2016-December/108559.html
https://reviews.llvm.org/D32313
Files:
clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tidy/modernize/UseEmplaceCheck.h
test/clang-tidy/modernize-use-emplace.cpp
Index: test/clang-tidy/modernize-use-emplace.cpp
===================================================================
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -186,6 +186,11 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
// CHECK-FIXES: v.emplace_back(1, 2);
+ v.push_back({1, 2});
+ v.push_back(std::pair<int, int>{1, 2});
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, 2);
+
GetPair g;
v.push_back(g.getPair());
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: clang-tidy/modernize/UseEmplaceCheck.h
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.h
+++ clang-tidy/modernize/UseEmplaceCheck.h
@@ -35,6 +35,7 @@
private:
std::vector<std::string> ContainersWithPushBack;
std::vector<std::string> SmartPointers;
+ std::vector<std::string> PairTypes;
};
} // namespace modernize
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -19,13 +19,16 @@
"::std::vector; ::std::list; ::std::deque";
static const auto DefaultSmartPointers =
"::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+static const auto DefaultPairTypes = "::std::pair";
UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
ContainersWithPushBack(utils::options::parseStringList(Options.get(
"ContainersWithPushBack", DefaultContainersWithPushBack))),
SmartPointers(utils::options::parseStringList(
- Options.get("SmartPointers", DefaultSmartPointers))) {}
+ Options.get("SmartPointers", DefaultSmartPointers))),
+ PairTypes(utils::options::parseStringList(
+ Options.get("PairTypes", DefaultPairTypes))) {}
void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
if (!getLangOpts().CPlusPlus11)
@@ -72,11 +75,19 @@
auto hasInitList = has(ignoringImplicit(initListExpr()));
// FIXME: Discard 0/NULL (as nullptr), static inline const data members,
// overloaded functions and template names.
+
+ // Ignore push_back({first, second}) for pair types (eg. std::pair).
+ auto isPairBraceInit = expr(allOf(cxxConstructExpr(hasDeclaration(
+ cxxConstructorDecl(ofClass(hasAnyName(SmallVector<StringRef, 2>(
+ PairTypes.begin(), PairTypes.end())))))),
+ unless(cxxTemporaryObjectExpr())),
+ unless(hasParent(implicitCastExpr())));
+
auto soughtConstructExpr =
cxxConstructExpr(
unless(anyOf(isCtorOfSmartPtr, hasInitList, bitFieldAsArgument,
initializerListAsArgument, newExprAsArgument,
- constructingDerived, isPrivateCtor)))
+ constructingDerived, isPrivateCtor, isPairBraceInit)))
.bind("ctor");
auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
Index: test/clang-tidy/modernize-use-emplace.cpp
===================================================================
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -186,6 +186,11 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
// CHECK-FIXES: v.emplace_back(1, 2);
+ v.push_back({1, 2});
+ v.push_back(std::pair<int, int>{1, 2});
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, 2);
+
GetPair g;
v.push_back(g.getPair());
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: clang-tidy/modernize/UseEmplaceCheck.h
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.h
+++ clang-tidy/modernize/UseEmplaceCheck.h
@@ -35,6 +35,7 @@
private:
std::vector<std::string> ContainersWithPushBack;
std::vector<std::string> SmartPointers;
+ std::vector<std::string> PairTypes;
};
} // namespace modernize
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -19,13 +19,16 @@
"::std::vector; ::std::list; ::std::deque";
static const auto DefaultSmartPointers =
"::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+static const auto DefaultPairTypes = "::std::pair";
UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
ContainersWithPushBack(utils::options::parseStringList(Options.get(
"ContainersWithPushBack", DefaultContainersWithPushBack))),
SmartPointers(utils::options::parseStringList(
- Options.get("SmartPointers", DefaultSmartPointers))) {}
+ Options.get("SmartPointers", DefaultSmartPointers))),
+ PairTypes(utils::options::parseStringList(
+ Options.get("PairTypes", DefaultPairTypes))) {}
void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
if (!getLangOpts().CPlusPlus11)
@@ -72,11 +75,19 @@
auto hasInitList = has(ignoringImplicit(initListExpr()));
// FIXME: Discard 0/NULL (as nullptr), static inline const data members,
// overloaded functions and template names.
+
+ // Ignore push_back({first, second}) for pair types (eg. std::pair).
+ auto isPairBraceInit = expr(allOf(cxxConstructExpr(hasDeclaration(
+ cxxConstructorDecl(ofClass(hasAnyName(SmallVector<StringRef, 2>(
+ PairTypes.begin(), PairTypes.end())))))),
+ unless(cxxTemporaryObjectExpr())),
+ unless(hasParent(implicitCastExpr())));
+
auto soughtConstructExpr =
cxxConstructExpr(
unless(anyOf(isCtorOfSmartPtr, hasInitList, bitFieldAsArgument,
initializerListAsArgument, newExprAsArgument,
- constructingDerived, isPrivateCtor)))
+ constructingDerived, isPrivateCtor, isPairBraceInit)))
.bind("ctor");
auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
