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

Piotr Padlewski via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 11 14:44:38 PDT 2016

Prazek added inline comments.

Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21
@@ +20,3 @@
+getHasAnyName(const std::vector<std::string> &names) {
+  llvm::Optional<ast_matchers::internal::Matcher<NamedDecl>> hasMatcher;
aaron.ballman wrote:
> aaron.ballman wrote:
> > Should be `Names` instead.
> Is this needed? We have the `hasAnyName()` matcher already.
The problem is that this matcher takes variadic number of strings. Here I have a vector of strings. sbenza did similar thing in performance-fast-string-find.

In the future I would like to add overload of the matcher that would take a vector of strings (it is is possible),
or at least move this function somewhere.

Another idea would be to return matcher instead of Optional<Matcher> - if list is empty then return unless(anything()) - there should be nothing() matcher :P

Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:31-33
@@ +30,5 @@
+const std::string defaultContainersWithPushBack =
+    "std::vector; std::list; std::deque";
+const std::string defaultSmartPointers =
+    "std::shared_ptr; std::unique_ptr; std::auto_ptr; std::weak_ptr";
aaron.ballman wrote:
> These should start with a capital as well (coding standard information pertaining to names can be found at http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly).
thanks, I always though that only local variables have to start with capital.



More information about the cfe-commits mailing list