alexfh requested changes to this revision. This revision now requires changes to proceed.
================ Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:31 @@ +30,3 @@ + +const std::string DefaultContainersWithPushBack = + "::std::vector; ::std::list; ::std::deque"; ---------------- A std::string is not needed here, please change to `const char []`. ================ Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36 @@ -32,1 +35,3 @@ +private: + std::vector<std::string> ContainersWithPushBack, SmartPointers; }; ---------------- Declarations with multiple variables can be misleading. Please split this into two separate declarations. ================ Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:7 @@ -6,3 +6,3 @@ This check looks for cases when inserting new element into an STL -container (``std::vector``, ``std::deque``, ``std::list``) or ``llvm::SmallVector`` -but the element is constructed temporarily. +container, but the element is constructed temporarily. + ---------------- IMO, this sentence fails to convey the most important parts of the information. Consider changing to something like this: > The check flags insertions to an STL-style container done by calling the > `push_back` method with an explicitly-constructed temporary of the container > element type. In this case, the corresponding `emplace_back` method results > in less verbose and potentially more efficient code. Maybe also explain, why the check only looks for `push_back`, and not `insert`, `push_front` and `push`. ================ Comment at: docs/clang-tidy/checks/modernize-use-emplace.rst:10 @@ +9,3 @@ +By default only ``std::vector``, ``std::deque``, ``std::list`` are considered. +This list can be modified by passing a `;` separated list of class names using the `ContainersWithPushBack` +option. ---------------- s/`;` separated/semicolon-separated/ Repository: rL LLVM https://reviews.llvm.org/D22208 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
