[PATCH] D22208: [clang-tidy] Fixes to modernize-use-emplace

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45
@@ -33,3 +44,3 @@
       hasDeclaration(functionDecl(hasName("push_back"))),
-      on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
-                                          "std::list", "std::deque")))));
+      on(hasType(cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
+          ContainersWithPushBack.begin(), ContainersWithPushBack.end()))))));
----------------
Prazek wrote:
> alexfh wrote:
> > What's the reason for explicitly creating a `SmallVector`? `VariadicFunction::operator()` takes an `ArrayRef`, which `std::vector<>` should be implicitly convertible to. Am I missing something?
> It is not. I guess it is because it is ArrayRef<StringRef> and I have std::vector<std::string>
Fair enough.

================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:112
@@ -95,1 +111,3 @@
+  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+      InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
 
----------------
> There is a bug here that I forgot about. Should be InnerCtorCall->getStartLoc()

Is this still relevant?

================
Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:12
@@ +11,3 @@
+uses it. It also doesn't support ``insert`` functions for associative containers
+because chaning ``insert`` to ``emplace`` may result in
+`speed regression <http://htmlpreview.github.io/?https://github.com/HowardHinnant/papers/blob/master/insert_vs_emplace.html>`_.
----------------
Prazek wrote:
> alexfh wrote:
> > alexfh wrote:
> > > s/chaning/changing/
> > How about the `insert` method of `std::vector<>`?
> insert -> emplace should be fine for the vector, but this patch is trying to mainly fix the bugs (and I want it to land in clang-3.9).
> 
> I will add ``insert`` near the ``push_front``
SG


Repository:
  rL LLVM

https://reviews.llvm.org/D22208