[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:27
+
+  return (ArgRef.str().length() > 0) ? ArgRef.str() + ")" : "()";
+}
----------------
couldn't this be 

```
return (!ArgRef.empty()) ? ArgRef.str() + ")" : "()";
```

You know I think there is a clang-tidy check for using empty() which was recently extended to handle length() too... I wonder if that would have fired


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:62
+    std::string diagText = "Perfer absl::WrapUnique for resetting unique_ptr";
+    std::string newText;
+
----------------
can we declare this when we create it see below?


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:72
+
+    newText =
+        ObjName.str() + " = absl::WrapUnique(" + getArgs(SM, callExpr) + ")";
----------------
std::string newText = ......


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:84
+
+    std::string diagText = "Perfer absl::WrapUnique to constructing unique_ptr";
+    std::string newText;
----------------
clang-tidy file in llvm/tools/clang says

key:             readability-identifier-naming.VariableCase
value:           CamelCase

this means that local variables like this should be DiagText and NewText (I think!)


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:85
+    std::string diagText = "Perfer absl::WrapUnique to constructing unique_ptr";
+    std::string newText;
+    std::string Left;
----------------
remove empty declarations until they are created


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:93
+
+    Left = (consDecl) ? "auto " + NameRef.str() + " = " : "";
+    newText = Left + "absl::WrapUnique(" + getArgs(SM, FC_Call) + ")";
----------------
std::string Left = (cons......


================
Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:94
+    Left = (consDecl) ? "auto " + NameRef.str() + " = " : "";
+    newText = Left + "absl::WrapUnique(" + getArgs(SM, FC_Call) + ")";
+    SourceLocation Target =
----------------
std::string newText = ....


================
Comment at: docs/ReleaseNotes.rst:83
+
+
 Improvements to include-fixer
----------------
remove double bank line 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435