[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:28
+  std::string FullNameTrimmed;
+  int Count = 0;
+  for (const auto &Character : FullName) {
----------------
Can you add a comment explaining what this loops tries to do? Ideally, with a short example like `some_func<int> --> ::some_func`


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:29
+  int Count = 0;
+  for (const auto &Character : FullName) {
+    if (Character == '<') {
----------------
Eugene.Zelenko wrote:
> I'm not sure, but probably braces could be elided in `for` and `if else`.
-1 for removing braces in multi-statement loops


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:46
+    } else if (FullNameTrimmedRef.endswith(Pattern) &&
+               FullNameTrimmedRef.drop_back(Pattern.size()).endswith("::")) {
+      return true;
----------------
Eugene.Zelenko wrote:
> I'm not sure, but probably braces could be elided.
-1


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:294
+  } else {
+    if ((MakeCall ? MakeCall->getNumArgs() : CtorCall->getNumArgs()) == 0) {
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
----------------
Can you pull this ternary expression into a variable so that it does not have to repeated when the diagnostic is emitted?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101471/new/

https://reviews.llvm.org/D101471