Prazek updated the summary for this revision.
Prazek updated this revision to Diff 61143.
http://reviews.llvm.org/D20964
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tidy/modernize/UseEmplaceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-emplace.rst
test/clang-tidy/modernize-use-emplace.cpp
Index: test/clang-tidy/modernize-use-emplace.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -0,0 +1,287 @@
+// RUN: %check_clang_tidy %s modernize-use-emplace %t
+
+namespace std {
+template <typename T>
+class vector {
+public:
+ void push_back(const T &) {}
+ void push_back(T &&) {}
+
+ template <typename... Args>
+ void emplace_back(Args &&... args){};
+};
+template <typename T>
+class list {
+public:
+ void push_back(const T &) {}
+ void push_back(T &&) {}
+
+ template <typename... Args>
+ void emplace_back(Args &&... args){};
+};
+
+template <typename T>
+class deque {
+public:
+ void push_back(const T &) {}
+ void push_back(T &&) {}
+
+ template <typename... Args>
+ void emplace_back(Args &&... args){};
+};
+
+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 <class U1, class U2>
+ pair(const pair<U1, U2> &p){};
+ template <class U1, class U2>
+ pair(pair<U1, U2> &&p){};
+};
+
+template <typename T1, typename T2>
+pair<T1, T2> make_pair(T1, T2) {
+ return pair<T1, T2>();
+};
+
+template <typename T>
+class unique_ptr {
+public:
+ unique_ptr(T *) {}
+};
+} // namespace std
+
+void testInts() {
+ std::vector<int> v;
+ v.push_back(42);
+ v.push_back(int(42));
+ v.push_back(int{42});
+ int z;
+ v.push_back(z);
+}
+
+struct S {
+ S(int a, int b = 41) {}
+ S() {}
+ void push_back(S);
+};
+
+struct Convertable {
+ operator S() { return S{}; }
+};
+
+struct Zoz {
+ Zoz(S s) {}
+};
+
+Zoz getZoz(S s) { return Zoz(s); }
+
+void test_S() {
+ std::vector<S> v;
+
+ v.push_back(S(1, 2));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of push_back [modernize-use-emplace]
+ // CHECK-FIXES: v.emplace_back(1, 2);
+
+ v.push_back(S{1, 2});
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, 2);
+
+ v.push_back(S());
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back();
+
+ v.push_back(S{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back();
+
+ v.push_back(42);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(42);
+
+ S temporary(42, 42);
+ temporary.push_back(temporary);
+ v.push_back(temporary);
+
+ v.push_back(Convertable());
+ v.push_back(Convertable{});
+ Convertable s;
+ v.push_back(s);
+}
+
+void test2() {
+ std::vector<Zoz> v;
+ v.push_back(Zoz(S(21, 37)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(S(21, 37));
+
+ v.push_back(getZoz(S(1, 2)));
+}
+
+void testPair() {
+ std::vector<std::pair<int, int>> v;
+ 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);
+
+ std::vector<std::pair<S, Zoz>> v2;
+ v2.push_back(std::pair<S, Zoz>(S(42, 42), Zoz(S(21, 37))));
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v2.emplace_back(S(42, 42), Zoz(S(21, 37)));
+}
+
+void testSpaces() {
+ std::vector<S> v;
+
+ // clang-format off
+
+ v.push_back(S(1, //arg1
+ 2 // arg2
+ ) // S
+ );
+ // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, //arg1
+ // CHECK-FIXES: 2 // arg2
+ // CHECK-FIXES: // S
+ // CHECK-FIXES: );
+
+ v.push_back( S (1, 2) );
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, 2 );
+ v.push_back( S {1, 2} );
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(1, 2 );
+
+ v.push_back( S {} );
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back( );
+
+ // clang-format on
+}
+
+void testPointers() {
+ std::vector<int *> v;
+ v.push_back(new int(5));
+
+ std::vector<std::unique_ptr<int>> v2;
+ v2.push_back(new int(42));
+ // This call can't be replaced with emplace_back.
+ // If emplacement will fail (not enough memory to add to vector)
+ // we will have leak of int because unique_ptr won't be constructed
+ // (and destructed) as in push_back case.
+
+ auto *ptr = new int;
+ v2.push_back(ptr);
+ // Same here
+}
+
+void testMakePair() {
+ std::vector<std::pair<int, int>> v;
+ // FIXME: add functionality to change calls of std::make_pair
+ v.push_back(std::make_pair(1, 2));
+
+ // FIXME: This is not a bug, but call of make_pair should be removed in the
+ // future. This one matches because the return type of make_pair is different
+ // than the pair itself.
+ v.push_back(std::make_pair(42LL, 13));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+}
+
+void testOtherCointainers() {
+ std::list<S> l;
+ l.push_back(S(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: l.emplace_back(42, 41);
+
+ std::deque<S> d;
+ d.push_back(S(42));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: d.emplace_back(42);
+}
+struct Aggregate {
+ int a, b;
+};
+
+void testAggregate() {
+ std::vector<Aggregate> v;
+ v.push_back(Aggregate({1, 2}));
+};
+
+class IntWrapper {
+public:
+ IntWrapper(int x) : value(x) {}
+ IntWrapper operator+(const IntWrapper other) const {
+ return IntWrapper(value + other.value);
+ }
+
+private:
+ int value;
+};
+
+void testMultipleOpsInPushBack() {
+ std::vector<IntWrapper> v;
+ v.push_back(IntWrapper(42) + IntWrapper(27));
+}
+
+// Macro tests.
+#define PUSH_BACK_WHOLE(c, x) c.push_back(x)
+#define PUSH_BACK_NAME push_back
+#define PUSH_BACK_ARG(x) (x)
+#define SOME_OBJ S(10)
+#define MILLION 3
+#define SOME_WEIRD_PUSH(v) v.push_back(S(
+#define OPEN (
+#define CLOSE )
+void macroTest() {
+ std::vector<S> v;
+ S s;
+
+ PUSH_BACK_WHOLE(v, S(5, 6));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use emplace_back
+
+ v.PUSH_BACK_NAME(S(5));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+ v.push_back PUSH_BACK_ARG(S(5, 6));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+ v.push_back(SOME_OBJ);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+ v.push_back(S(MILLION));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // CHECK-FIXES: v.emplace_back(MILLION);
+
+ // clang-format off
+ v.push_back( S OPEN 3 CLOSE );
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+ // clang-format on
+ PUSH_BACK_WHOLE(s, S(1));
+}
+
+struct A {
+ int value1, value2;
+};
+
+struct B {
+ B(A) {}
+};
+
+struct C {
+ int value1, value2, value3;
+};
+
+void testAggregation() {
+ // This should not be noticed or fixed; after the correction, the code won't
+ // compile.
+ std::vector<B> v;
+ v.push_back(B({10, 42}));
+}
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -0,0 +1,60 @@
+.. title:: clang-tidy - modernize-use-emplace
+
+modernize-use-emplace
+=====================
+
+This check looks for cases when inserting new element into an STL
+container, but the element is constructed temporarily.
+
+Before:
+
+.. code:: c++
+
+ std::vector<MyClass> v;
+ v.push_back(MyClass(21, 37));
+
+ std::vector<std::pair<int,int> > w;
+ w.push_back(std::make_pair(21, 37));
+
+After:
+
+.. code:: c++
+
+ std::vector<MyClass> v;
+ v.emplace_back(21, 37);
+
+ std::vector<std::pair<int,int> > w;
+ v.emplace_back(21, 37);
+
+The other situation is when we pass arguments that will be converted to a type
+inside a container.
+
+Before:
+
+.. code:: c++
+
+ std::vector<boost::optional<std::string> > v;
+ v.push_back("abc");
+
+After:
+
+.. code:: c++
+
+ std::vector<boost::optional<std::string> > v;
+ v.emplace_back("abc");
+
+
+In this case the calls of ``push_back`` won't be replaced.
+
+.. code:: c++
+ std::vector<std::unique_ptr<int> > v;
+ v.push_back(new int(5));
+ auto *ptr = int;
+ v.push_back(ptr);
+
+This is because replacing it with ``emplace_back`` could cause a leak of this
+pointer if ``emplace_back`` would throw exception before emplacement
+(e.g. not enough memory to add new element).
+
+For more info read item 42 - "Consider emplacement instead of insertion."
+of Scott Meyers Efective Modern C++.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -104,6 +104,7 @@
modernize-use-auto
modernize-use-bool-literals
modernize-use-default
+ modernize-use-emplace
modernize-use-nullptr
modernize-use-override
performance-faster-string-find
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -206,6 +206,11 @@
Finds integer literals which are cast to ``bool``.
+- New `modernize-use-emplace
+ <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
+
+ Finds calls that could be changed to emplace.
+
- New `performance-faster-string-find
<http://clang.llvm.org/extra/clang-tidy/checks/performance-faster-string-find.html>`_ check
Index: clang-tidy/modernize/UseEmplaceCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseEmplaceCheck.h
@@ -0,0 +1,38 @@
+//===--- UseEmplaceCheck.h - clang-tidy--------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// This check looks for cases when inserting new element into std::vector but
+/// the element is constructed temporarily.
+/// It replaces those calls for emplace_back of arguments passed to
+/// constructor of temporary object.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html
+class UseEmplaceCheck : public ClangTidyCheck {
+public:
+ UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -0,0 +1,91 @@
+//===--- UseEmplaceCheck.cpp - clang-tidy----------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseEmplaceCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus11)
+ return;
+
+ // FIXME: Bunch of functionality that could be easily added:
+ // + add handling of `push_front` for std::forward_list, std::list
+ // and std::deque.
+ // + add handling of `push` for std::stack, std::queue, std::priority_queue
+ // + add handling of `insert` for stl associative container, but be careful
+ // because this requires special treatment (it could cause performance
+ // regression)
+ // + match for emplace calls that should be replaced with insertion
+ // + match for make_pair calls.
+ auto callPushBack = cxxMemberCallExpr(
+ hasDeclaration(functionDecl(hasName("push_back"))),
+ on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
+ "std::list", "std::deque")))));
+
+ // 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
+ // (and destructed) as in push_back case.
+ auto isCtorOfSmartPtr = hasDeclaration(cxxConstructorDecl(
+ ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr", "std::auto_ptr",
+ "std::weak_ptr"))));
+
+ auto hasConstructExpr = has(ignoringParenImpCasts(
+ cxxConstructExpr(
+ unless(anyOf(isCtorOfSmartPtr, has(initListExpr()),
+ has(materializeTemporaryExpr(has(initListExpr()))))))
+ .bind("ctor")));
+
+ auto ctorAsArgument = materializeTemporaryExpr(
+ anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr))));
+
+ Finder->addMatcher(
+ cxxMemberCallExpr(callPushBack, has(ctorAsArgument)).bind("call"), this);
+}
+
+void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ const auto *InnerCtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+
+ auto FunctionNameSourceRange = CharSourceRange::getCharRange(
+ Call->getExprLoc(), Call->getArg(0)->getExprLoc());
+
+ auto Diag = diag(Call->getExprLoc(), "use emplace_back instead of push_back");
+
+ if (FunctionNameSourceRange.getBegin().isMacroID())
+ return;
+
+ Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+ "emplace_back(");
+
+ auto CallParensRange = InnerCtorCall->getParenOrBraceRange();
+
+ // Finish if there is no explicit constructor call.
+ if (CallParensRange.getBegin().isInvalid())
+ return;
+
+ // Range for constructor name and opening brace.
+ auto CtorCallSourceRange = CharSourceRange::getCharRange(
+ InnerCtorCall->getExprLoc(),
+ CallParensRange.getBegin().getLocWithOffset(1));
+
+ Diag << FixItHint::CreateRemoval(CtorCallSourceRange)
+ << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ CallParensRange.getEnd(),
+ CallParensRange.getEnd().getLocWithOffset(1)));
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -23,6 +23,7 @@
#include "UseAutoCheck.h"
#include "UseBoolLiteralsCheck.h"
#include "UseDefaultCheck.h"
+#include "UseEmplaceCheck.h"
#include "UseNullptrCheck.h"
#include "UseOverrideCheck.h"
@@ -54,6 +55,7 @@
CheckFactories.registerCheck<UseBoolLiteralsCheck>(
"modernize-use-bool-literals");
CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
+ CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
}
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -17,6 +17,7 @@
UseAutoCheck.cpp
UseBoolLiteralsCheck.cpp
UseDefaultCheck.cpp
+ UseEmplaceCheck.cpp
UseNullptrCheck.cpp
UseOverrideCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
