alexfh added inline comments.

================
Comment at: clang-tidy/performance/InefficientVectorOperationCheck.cpp:208
+           "consider pre-allocating the vector capacity before the loop")
+      << VectorAppendCall->getMethodDecl()->getDeclName();
 
----------------
hokein wrote:
> alexfh wrote:
> > Diagnostic builder should be able to format NamedDecls directly, this 
> > `->getDeclName()` is not necessary. The only difference is that it will 
> > likely add quotes around the name, which seems to be good anyway.
> I tried it, but I found the behavior between using `getDeclName()` and not 
> using `getDeclName()` is different when handling the template functions:
> 
> * `diag(...) << VectorAppendCall->getMethodDecl()` will print the function 
> name with instantiated template arguments like "emplace_back<int&>";
> *  `diag(...) << VectorAppendCall->getMethodDecl()->getDeclName()` will just 
> print the function name without template arguments, which is what we expect.
Good to know.


================
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";
----------------
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.


================
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) {
----------------
This pattern is ambiguous. I'd use unique variable name for each test to avoid 
patterns matching incorrect lines.


https://reviews.llvm.org/D33209



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to