[PATCH] D33209: [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:50
+static const char PushBackOrEmplaceBackCallName[] =
+    "push_back_or_emplace_back_call";
 static const char LoopInitVarName[] = "loop_init_var";
----------------
alexfh wrote:
> nit: No need for the actual strings to be that long, since you're only using the corresponding constant names in the code. Even though there's no small string optimization used for bound AST nodes (they are stored in a `std::map<std::string, ast_type_traits::DynTypedNode>`), maps are frequently compared, which is less wasteful with shorter strings.
> 
> It would be interesting to see how large is the impact of longer or shorter string IDs, but until then we can still avoid overly long names.
Acknowledged. Thanks for the detailed explanation ;)


================
Comment at: test/clang-tidy/performance-inefficient-vector-operation.cpp:158
+    std::vector<Foo> v;
+    // CHECK-FIXES: v.reserve(t.size());
+    for (const auto &e : t) {
----------------
alexfh wrote:
> This pattern is ambiguous. I'd use unique variable name for each test to avoid patterns matching incorrect lines.
Sounds good. Also applied the change to the whole test file.


https://reviews.llvm.org/D33209