nicovank updated this revision to Diff 341342.
nicovank added a comment.
Fix a couple variable names and explicitely specify a couple types
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101471/new/
https://reviews.llvm.org/D101471
Files:
clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
@@ -10,15 +10,36 @@
namespace std {
template <typename>
-class initializer_list
-{
+class initializer_list {
public:
initializer_list() noexcept {}
};
+template <typename T1, typename T2>
+class pair {
+public:
+ pair() = default;
+ pair(const pair &) = default;
+ pair(pair &&) = default;
+
+ pair(const T1 &, const T2 &) {}
+ pair(T1 &&, T2 &&) {}
+
+ template <typename U1, typename U2>
+ pair(const pair<U1, U2> &){};
+ template <typename U1, typename U2>
+ pair(pair<U1, U2> &&){};
+};
+
template <typename T>
class vector {
public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
vector() = default;
vector(initializer_list<T>) {}
@@ -26,55 +47,231 @@
void push_back(T &&) {}
template <typename... Args>
- void emplace_back(Args &&... args){};
+ void emplace_back(Args &&...args){};
+ template <typename... Args>
+ iterator emplace(const_iterator pos, Args &&...args){};
~vector();
};
+
template <typename T>
class list {
public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
void push_back(const T &) {}
void push_back(T &&) {}
template <typename... Args>
- void emplace_back(Args &&... args){};
+ iterator emplace(const_iterator pos, Args &&...args){};
+ template <typename... Args>
+ void emplace_back(Args &&...args){};
+ template <typename... Args>
+ void emplace_front(Args &&...args){};
~list();
};
template <typename T>
class deque {
public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
void push_back(const T &) {}
void push_back(T &&) {}
template <typename... Args>
- void emplace_back(Args &&... args){};
+ iterator emplace(const_iterator pos, Args &&...args){};
+ template <typename... Args>
+ void emplace_back(Args &&...args){};
+ template <typename... Args>
+ void emplace_front(Args &&...args){};
~deque();
};
-template <typename T> struct remove_reference { using type = T; };
-template <typename T> struct remove_reference<T &> { using type = T; };
-template <typename T> struct remove_reference<T &&> { using type = T; };
+template <typename T>
+class forward_list {
+public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace_front(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_after(const_iterator pos, Args &&...args){};
+};
-template <typename T1, typename T2> class pair {
+template <typename T>
+class set {
public:
- pair() = default;
- pair(const pair &) = default;
- pair(pair &&) = default;
+ using value_type = T;
- pair(const T1 &, const T2 &) {}
- pair(T1 &&, T2 &&) {}
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class map {
+public:
+ using value_type = std::pair<Key, T>;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class multiset {
+public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class multimap {
+public:
+ using value_type = std::pair<Key, T>;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class unordered_set {
+public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
- template <typename U1, typename U2> pair(const pair<U1, U2> &){};
- template <typename U1, typename U2> pair(pair<U1, U2> &&){};
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class unordered_map {
+public:
+ using value_type = std::pair<Key, T>;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class unordered_multiset {
+public:
+ using value_type = T;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class unordered_multimap {
+public:
+ using value_type = std::pair<Key, T>;
+
+ class iterator {};
+ class const_iterator {};
+ const_iterator begin() { return const_iterator{}; }
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+ template <typename... Args>
+ iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class stack {
+public:
+ using value_type = T;
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+};
+
+template <typename T>
+class queue {
+public:
+ using value_type = T;
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
+};
+
+template <typename T>
+class priority_queue {
+public:
+ using value_type = T;
+
+ template <typename... Args>
+ void emplace(Args &&...args){};
};
+template <typename T>
+struct remove_reference { using type = T; };
+template <typename T>
+struct remove_reference<T &> { using type = T; };
+template <typename T>
+struct remove_reference<T &&> { using type = T; };
+
template <typename T1, typename T2>
pair<typename remove_reference<T1>::type, typename remove_reference<T2>::type>
make_pair(T1 &&, T2 &&) {
return {};
};
-template <typename... Ts> class tuple {
+template <typename... Ts>
+class tuple {
public:
tuple() = default;
tuple(const tuple &) = default;
@@ -83,13 +280,17 @@
tuple(const Ts &...) {}
tuple(Ts &&...) {}
- template <typename... Us> tuple(const tuple<Us...> &){};
- template <typename... Us> tuple(tuple<Us...> &&) {}
+ template <typename... Us>
+ tuple(const tuple<Us...> &){};
+ template <typename... Us>
+ tuple(tuple<Us...> &&) {}
- template <typename U1, typename U2> tuple(const pair<U1, U2> &) {
+ template <typename U1, typename U2>
+ tuple(const pair<U1, U2> &) {
static_assert(sizeof...(Ts) == 2, "Wrong tuple size");
};
- template <typename U1, typename U2> tuple(pair<U1, U2> &&) {
+ template <typename U1, typename U2>
+ tuple(pair<U1, U2> &&) {
static_assert(sizeof...(Ts) == 2, "Wrong tuple size");
};
};
@@ -115,10 +316,10 @@
void push_back(T &&) {}
template <typename... Args>
- void emplace_back(Args &&... args){};
+ void emplace_back(Args &&...args){};
};
-} // llvm
+} // namespace llvm
void testInts() {
std::vector<int> v;
@@ -375,7 +576,7 @@
// make_pair cannot be removed here, as X is not constructible with two ints.
struct Y {
- Y(std::pair<int, int>&&) {}
+ Y(std::pair<int, int> &&) {}
};
std::vector<Y> y;
y.push_back(std::make_pair(2, 3));
@@ -402,7 +603,8 @@
}
namespace test {
-template <typename T> struct Single {
+template <typename T>
+struct Single {
Single() = default;
Single(const Single &) = default;
Single(Single &&) = default;
@@ -410,11 +612,15 @@
Single(const T &) {}
Single(T &&) {}
- template <typename U> Single(const Single<U> &) {}
- template <typename U> Single(Single<U> &&) {}
+ template <typename U>
+ Single(const Single<U> &) {}
+ template <typename U>
+ Single(Single<U> &&) {}
- template <typename U> Single(const std::tuple<U> &) {}
- template <typename U> Single(std::tuple<U> &&) {}
+ template <typename U>
+ Single(const std::tuple<U> &) {}
+ template <typename U>
+ Single(std::tuple<U> &&) {}
};
template <typename T>
@@ -605,3 +811,159 @@
x.push_back(PairIntVector(3, {4}));
x.push_back({5, {6}});
}
+
+class Foo {
+public:
+ Foo(){};
+ Foo(int){};
+ Foo(int, int){};
+ Foo(std::pair<int, int>){};
+
+protected:
+ Foo(char *) : Foo(){};
+};
+
+void testSomeEmplaceCases() {
+ std::vector<std::pair<char *, char *>> v1;
+ std::vector<Foo> v2;
+ std::unordered_map<int, char *> m1;
+
+ // CHECK-FIXES: v1.emplace_back("foo", "bar");
+ // CHECK-FIXES: v1.emplace_back(foo, "bar");
+ // CHECK-FIXES: v1.emplace(v1.begin(), "foo", "bar");
+ char *foo = "bar";
+ v1.emplace_back(std::make_pair("foo", "bar"));
+ v1.emplace_back(std::make_pair(foo, "bar"));
+ v1.emplace(v1.begin(), std::make_pair("foo", "bar"));
+
+ // CHECK-FIXES: v2.emplace_back();
+ // CHECK-FIXES: v2.emplace_back(13);
+ // CHECK-FIXES: v2.emplace_back(13, a);
+ int a = 31;
+ v2.emplace_back(Foo());
+ v2.emplace_back(Foo(13));
+ v2.emplace_back(Foo(13, a));
+
+ // CHECK-MESSAGES-NOT: [[@LINE+1]]:19: warning: unnecessary temporary object created while calling emplace_back
+ v2.emplace_back(std::make_pair(3, 3));
+
+ // CHECK-FIXES: m1.emplace(13, "foo");
+ m1.emplace(std::make_pair(13, "foo"));
+}
+
+void testAllSTLEmplacyFunctions() {
+ std::vector<Foo> vector;
+ std::deque<Foo> deque;
+ std::forward_list<Foo> forward_list;
+ std::list<Foo> list;
+ std::set<Foo> set;
+ std::map<int, Foo> map;
+ std::multiset<Foo> multiset;
+ std::multimap<int, Foo> multimap;
+ std::unordered_set<Foo> unordered_set;
+ std::unordered_map<int, Foo> unordered_map;
+ std::unordered_multiset<Foo> unordered_multiset;
+ std::unordered_multimap<int, Foo> unordered_multimap;
+ std::stack<Foo> stack;
+ std::queue<Foo> queue;
+ std::priority_queue<Foo> priority_queue;
+
+ // CHECK-FIXES: vector.emplace_back(13);
+ // CHECK-FIXES: vector.emplace(vector.begin(), 13);
+ vector.emplace_back(Foo(13));
+ vector.emplace(vector.begin(), Foo(13));
+
+ // CHECK-FIXES: deque.emplace(deque.begin(), 13);
+ // CHECK-FIXES: deque.emplace_front(13);
+ // CHECK-FIXES: deque.emplace_back(13);
+ deque.emplace(deque.begin(), Foo(13));
+ deque.emplace_front(Foo(13));
+ deque.emplace_back(Foo(13));
+
+ // CHECK-FIXES: forward_list.emplace_after(forward_list.begin(), 13);
+ // CHECK-FIXES: forward_list.emplace_front(13);
+ forward_list.emplace_after(forward_list.begin(), Foo(13));
+ forward_list.emplace_front(Foo(13));
+
+ // CHECK-FIXES: list.emplace(list.begin(), 13);
+ // CHECK-FIXES: list.emplace_back(13);
+ // CHECK-FIXES: list.emplace_front(13);
+ list.emplace(list.begin(), Foo(13));
+ list.emplace_back(Foo(13));
+ list.emplace_front(Foo(13));
+
+ // CHECK-FIXES: set.emplace(13);
+ // CHECK-FIXES: set.emplace_hint(set.begin(), 13);
+ set.emplace(Foo(13));
+ set.emplace_hint(set.begin(), Foo(13));
+
+ // CHECK-FIXES: map.emplace(13, Foo(13));
+ // CHECK-FIXES: map.emplace_hint(map.begin(), 13, Foo(13));
+ map.emplace(std::make_pair(13, Foo(13)));
+ map.emplace_hint(map.begin(), std::make_pair(13, Foo(13)));
+
+ // CHECK-FIXES: multiset.emplace(13);
+ // CHECK-FIXES: multiset.emplace_hint(multiset.begin(), 13);
+ multiset.emplace(Foo(13));
+ multiset.emplace_hint(multiset.begin(), Foo(13));
+
+ // CHECK-FIXES: multimap.emplace(13, Foo(13));
+ // CHECK-FIXES: multimap.emplace_hint(multimap.begin(), 13, Foo(13));
+ multimap.emplace(std::make_pair(13, Foo(13)));
+ multimap.emplace_hint(multimap.begin(), std::make_pair(13, Foo(13)));
+
+ // CHECK-FIXES: unordered_set.emplace(13);
+ // CHECK-FIXES: unordered_set.emplace_hint(unordered_set.begin(), 13);
+ unordered_set.emplace(Foo(13));
+ unordered_set.emplace_hint(unordered_set.begin(), Foo(13));
+
+ // CHECK-FIXES: unordered_map.emplace(13, Foo(13));
+ // CHECK-FIXES: unordered_map.emplace_hint(unordered_map.begin(), 13, Foo(13));
+ unordered_map.emplace(std::make_pair(13, Foo(13)));
+ unordered_map.emplace_hint(unordered_map.begin(), std::make_pair(13, Foo(13)));
+
+ // CHECK-FIXES: unordered_multiset.emplace(13);
+ // CHECK-FIXES: unordered_multiset.emplace_hint(unordered_multiset.begin(), 13);
+ unordered_multiset.emplace(Foo(13));
+ unordered_multiset.emplace_hint(unordered_multiset.begin(), Foo(13));
+
+ // CHECK-FIXES: unordered_multimap.emplace(13, Foo(13));
+ // CHECK-FIXES: unordered_multimap.emplace_hint(unordered_multimap.begin(), 13, Foo(13));
+ unordered_multimap.emplace(std::make_pair(13, Foo(13)));
+ unordered_multimap.emplace_hint(unordered_multimap.begin(), std::make_pair(13, Foo(13)));
+
+ // CHECK-FIXES: stack.emplace(13);
+ // CHECK-FIXES: queue.emplace(13);
+ // CHECK-FIXES: priority_queue.emplace(13);
+ stack.emplace(Foo(13));
+ queue.emplace(Foo(13));
+ priority_queue.emplace(Foo(13));
+}
+
+struct Bar {
+public:
+ Bar(){};
+ void testWithPrivateAndProtectedCtor() {
+ std::vector<Bar> vec;
+ // CHECK-MESSAGES-NOT: [[@LINE+1]]:22: warning: unnecessary temporary object created while calling emplace_back
+ vec.emplace_back(Bar(13));
+ // CHECK-MESSAGES-NOT: [[@LINE+1]]:22: warning: unnecessary temporary object created while calling emplace_back
+ vec.emplace_back(Bar(13, 13));
+ }
+
+protected:
+ Bar(int){};
+
+private:
+ Bar(int, int){};
+};
+
+void testMoreFalsePositiveCases() {
+ std::unordered_map<int, std::pair<char *, int>> map;
+ // CHECK-MESSAGES-NOT: [[@LINE+1]]:19: warning: unnecessary temporary object created while calling emplace
+ map.emplace(13, std::make_pair("foo", 13));
+
+ std::vector<std::tuple<std::tuple<int, int>>> vec;
+ // CHECK-MESSAGES-NOT: [[@LINE+1]]:20: warning: unnecessary temporary object created while calling emplace_back
+ vec.emplace_back(std::make_tuple(13, 31));
+}
Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
@@ -40,6 +40,7 @@
const std::vector<std::string> SmartPointers;
const std::vector<std::string> TupleTypes;
const std::vector<std::string> TupleMakeFunctions;
+ const std::vector<std::string> EmplacyFunctions;
};
} // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,66 @@
namespace modernize {
namespace {
+// Identical to hasAnyName, except it does not take template specifiers into
+// account. This is used to match the functions names as in
+// DefaultEmplacyFunctions below without caring about the template types of the
+// containers. Inspired from HasNameMatcher::matchesNodeFullSlow in
+// clang/lib/ASTMatchers/ASTMatchersInternal.cpp.
+AST_MATCHER_P(NamedDecl, hasAnyNameIgnoringTemplates, std::vector<std::string>,
+ Names) {
+ const std::string FullName = "::" + Node.getQualifiedNameAsString();
+
+ std::string FullNameTrimmed;
+ int Count = 0;
+ for (const auto &Character : FullName) {
+ if (Character == '<') {
+ ++Count;
+ } else if (Character == '>') {
+ --Count;
+ } else if (Count == 0) {
+ FullNameTrimmed.append(1, Character);
+ }
+ }
+
+ const StringRef FullNameTrimmedRef = StringRef(FullNameTrimmed);
+
+ for (const StringRef Pattern : Names) {
+ if (Pattern.startswith("::")) {
+ if (FullNameTrimmed == Pattern)
+ return true;
+ } else if (FullNameTrimmedRef.endswith(Pattern) &&
+ FullNameTrimmedRef.drop_back(Pattern.size()).endswith("::")) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+// Checks if the given matcher is the last argument of the given CallExpr.
+AST_MATCHER_P(CallExpr, hasLastArgument,
+ clang::ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+ if (Node.getNumArgs() == 0) {
+ return false;
+ }
+
+ return InnerMatcher.matches(*Node.getArg(Node.getNumArgs() - 1), Finder,
+ Builder);
+}
+
+// Checks if the given member call has the same number of arguments as the
+// function had parameters defined (this is useful to check if there is only one
+// variadic argument).
+AST_MATCHER(CXXMemberCallExpr, hasSameNumArgsAsDeclNumParams) {
+ if (Node.getMethodDecl()->isFunctionTemplateSpecialization()) {
+ return Node.getNumArgs() == Node.getMethodDecl()
+ ->getPrimaryTemplate()
+ ->getTemplatedDecl()
+ ->getNumParams();
+ }
+ return Node.getNumArgs() == Node.getMethodDecl()->getNumParams();
+}
+
AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
return Node.hasExplicitTemplateArgs();
}
@@ -25,6 +85,20 @@
"::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
const auto DefaultTupleTypes = "::std::pair; ::std::tuple";
const auto DefaultTupleMakeFunctions = "::std::make_pair; ::std::make_tuple";
+const auto DefaultEmplacyFunctions =
+ "vector::emplace_back; vector::emplace;"
+ "deque::emplace; deque::emplace_front; deque::emplace_back;"
+ "forward_list::emplace_after; forward_list::emplace_front;"
+ "list::emplace; list::emplace_back; list::emplace_front;"
+ "set::emplace; set::emplace_hint;"
+ "map::emplace; map::emplace_hint;"
+ "multiset::emplace; multiset::emplace_hint;"
+ "multimap::emplace; multimap::emplace_hint;"
+ "unordered_set::emplace; unordered_set::emplace_hint;"
+ "unordered_map::emplace; unordered_map::emplace_hint;"
+ "unordered_multiset::emplace; unordered_multiset::emplace_hint;"
+ "unordered_multimap::emplace; unordered_multimap::emplace_hint;"
+ "stack::emplace; queue::emplace; priority_queue::emplace";
} // namespace
UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
@@ -37,7 +111,9 @@
TupleTypes(utils::options::parseStringList(
Options.get("TupleTypes", DefaultTupleTypes))),
TupleMakeFunctions(utils::options::parseStringList(
- Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))) {}
+ Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))),
+ EmplacyFunctions(utils::options::parseStringList(
+ Options.get("EmplacyFunctions", DefaultEmplacyFunctions))) {}
void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
// FIXME: Bunch of functionality that could be easily added:
@@ -53,6 +129,13 @@
on(hasType(cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
ContainersWithPushBack.begin(), ContainersWithPushBack.end()))))));
+ auto CallEmplacy = cxxMemberCallExpr(
+ hasDeclaration(
+ functionDecl(hasAnyNameIgnoringTemplates(EmplacyFunctions))),
+ on(hasType(cxxRecordDecl(has(typedefNameDecl(
+ hasName("value_type"), hasType(type(hasUnqualifiedDesugaredType(
+ recordType().bind("value_type"))))))))));
+
// We can't replace push_backs of smart pointer because
// if emplacement fails (f.e. bad_alloc in vector) we will have leak of
// passed pointer because smart pointer won't be constructed
@@ -74,8 +157,9 @@
auto ConstructingDerived =
hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
- // emplace_back can't access private constructor.
- auto IsPrivateCtor = hasDeclaration(cxxConstructorDecl(isPrivate()));
+ // emplace_back can't access private or protected constructors.
+ auto IsPrivateOrProtectedCtor =
+ hasDeclaration(cxxConstructorDecl(anyOf(isPrivate(), isProtected())));
auto HasInitList = anyOf(has(ignoringImplicit(initListExpr())),
has(cxxStdInitializerListExpr()));
@@ -86,7 +170,7 @@
cxxConstructExpr(
unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument,
InitializerListAsArgument, NewExprAsArgument,
- ConstructingDerived, IsPrivateCtor)))
+ ConstructingDerived, IsPrivateOrProtectedCtor)))
.bind("ctor");
auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
@@ -106,22 +190,63 @@
SmallVector<StringRef, 2>(TupleTypes.begin(), TupleTypes.end())))))));
auto SoughtParam = materializeTemporaryExpr(
- anyOf(has(MakeTuple), has(MakeTupleCtor),
- HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr))));
+ anyOf(has(MakeTuple), has(MakeTupleCtor), HasConstructExpr,
+ has(cxxFunctionalCastExpr(HasConstructExpr))));
+
+ auto HasConstructExprWithValueTypeType =
+ has(ignoringImplicit(cxxConstructExpr(
+ SoughtConstructExpr, hasType(type(equalsBoundNode("value_type"))))));
+
+ auto HasConstructExprWithValueTypeTypeAsLastArgument =
+ hasLastArgument(materializeTemporaryExpr(anyOf(
+ HasConstructExprWithValueTypeType,
+ has(cxxFunctionalCastExpr(HasConstructExprWithValueTypeType)))));
Finder->addMatcher(
traverse(TK_AsIs, cxxMemberCallExpr(CallPushBack, has(SoughtParam),
unless(isInTemplateInstantiation()))
- .bind("call")),
+ .bind("push_back_call")),
+ this);
+
+ Finder->addMatcher(
+ traverse(TK_AsIs,
+ cxxMemberCallExpr(
+ CallEmplacy, HasConstructExprWithValueTypeTypeAsLastArgument,
+ hasSameNumArgsAsDeclNumParams(),
+ unless(isInTemplateInstantiation()))
+ .bind("emplacy_call")),
+ this);
+
+ Finder->addMatcher(
+ traverse(
+ TK_AsIs,
+ cxxMemberCallExpr(
+ CallEmplacy,
+ on(hasType(cxxRecordDecl(has(typedefNameDecl(
+ hasName("value_type"),
+ hasType(type(
+ hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+ cxxRecordDecl(hasAnyName(SmallVector<StringRef, 2>(
+ TupleTypes.begin(), TupleTypes.end()))))))))))))),
+ has(MakeTuple), hasSameNumArgsAsDeclNumParams(),
+ unless(isInTemplateInstantiation()))
+ .bind("emplacy_call")),
this);
}
void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ const auto *PushBackCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_back_call");
+ const auto *EmplacyCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
+
+ assert((PushBackCall || EmplacyCall) && "No call matched");
assert((CtorCall || MakeCall) && "No push_back parameter matched");
+ const auto *Call = PushBackCall ? PushBackCall : EmplacyCall;
+
if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
return;
@@ -129,13 +254,21 @@
const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
Call->getExprLoc(), Call->getArg(0)->getExprLoc());
- auto Diag = diag(Call->getExprLoc(), "use emplace_back instead of push_back");
+ auto Diag =
+ PushBackCall
+ ? diag(Call->getExprLoc(), "use emplace_back instead of push_back")
+ : diag(MakeCall ? MakeCall->getBeginLoc() : CtorCall->getBeginLoc(),
+ "unnecessary temporary object created while calling " +
+ Call->getMethodDecl()->getName().str());
if (FunctionNameSourceRange.getBegin().isMacroID())
return;
- const auto *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
- Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, EmplacePrefix);
+ if (PushBackCall) {
+ const auto *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
+ Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+ EmplacePrefix);
+ }
const SourceRange CallParensRange =
MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
@@ -146,16 +279,31 @@
if (CallParensRange.getBegin().isInvalid())
return;
- const SourceLocation ExprBegin =
- MakeCall ? MakeCall->getExprLoc() : CtorCall->getExprLoc();
+ if (PushBackCall) {
+ const SourceLocation ExprBegin =
+ MakeCall ? MakeCall->getExprLoc() : CtorCall->getExprLoc();
- // Range for constructor name and opening brace.
- const auto ParamCallSourceRange =
- CharSourceRange::getTokenRange(ExprBegin, CallParensRange.getBegin());
+ // Range for constructor name and opening brace.
+ const auto ParamCallSourceRange =
+ CharSourceRange::getTokenRange(ExprBegin, CallParensRange.getBegin());
- Diag << FixItHint::CreateRemoval(ParamCallSourceRange)
- << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- CallParensRange.getEnd(), CallParensRange.getEnd()));
+ Diag << FixItHint::CreateRemoval(ParamCallSourceRange)
+ << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ CallParensRange.getEnd(), CallParensRange.getEnd()));
+ } else {
+ if ((MakeCall ? MakeCall->getNumArgs() : CtorCall->getNumArgs()) == 0) {
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ MakeCall ? MakeCall->getSourceRange() : CtorCall->getSourceRange()));
+ } else {
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ MakeCall ? MakeCall->getBeginLoc() : CtorCall->getBeginLoc(),
+ MakeCall ? MakeCall->getArg(0)->getBeginLoc()
+ : CtorCall->getArg(0)->getBeginLoc()))
+ << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ MakeCall ? MakeCall->getRParenLoc() : CtorCall->getEndLoc(),
+ MakeCall ? MakeCall->getRParenLoc() : CtorCall->getEndLoc()));
+ }
+ }
}
void UseEmplaceCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -168,6 +316,8 @@
utils::options::serializeStringList(TupleTypes));
Options.store(Opts, "TupleMakeFunctions",
utils::options::serializeStringList(TupleMakeFunctions));
+ Options.store(Opts, "EmplacyFunctions",
+ utils::options::serializeStringList(EmplacyFunctions));
}
} // namespace modernize
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
