[PATCH] D25316: [clang-tidy] Enhance modernize-use-auto to casts

[Malcolm Parsons via cfe-commits]

malcolm.parsons added inline comments.


================
Comment at: clang-tidy/modernize/UseAutoCheck.cpp:346
 
-    const auto *NewExpr = cast<CXXNewExpr>(V->getInit()->IgnoreParenImpCasts());
-    // Ensure that every VarDecl has a CXXNewExpr initializer.
-    if (!NewExpr)
+    const auto *Expr = cast<ExprType>(V->getInit()->IgnoreParenImpCasts());
+    // Ensure that every VarDecl has an ExprType initializer.
----------------
aaron.ballman wrote:
> I think you should use `dyn_cast` here because `ExprType` may vary from call to call and produce an invalid cast otherwise.
The `cast` was safe in the existing check because `makeDeclWithNewMatcher` creates a matcher that ensures all initializers are `CxxNewExpr`. 
`makeDeclWithCastMatcher` needs to be fixed.


================
Comment at: clang-tidy/modernize/UseAutoCheck.cpp:404
                  Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId)) {
-    replaceNew(Decl, Result.Context);
+    auto GetType = [](const CXXNewExpr *Expr) {
+      return Expr->getType();
----------------
aaron.ballman wrote:
> Just inline the lambda expression (below as well).
OK.


================
Comment at: clang-tidy/modernize/UseAutoCheck.h:30
+  void replaceExpr(const DeclStmt *D, ASTContext *Context, TypeFn GetType,
+                   StringRef message);
 
----------------
aaron.ballman wrote:
> s/message/Message
Oops.


================
Comment at: docs/clang-tidy/checks/modernize-use-auto.rst:150
+Frequently, when a variable is declared and initialized with a cast, the
+variable type has to be written twice: in the declaration type and in the
+cast expression. In this cases, the declaration type can be replaced with
----------------
aaron.ballman wrote:
> s/has to be/is
It's a copy and paste from the section above; I'll fix both.


================
Comment at: docs/clang-tidy/checks/modernize-use-auto.rst:164-165
+``reinterpret_cast``, functional casts and c style casts.
+It does not handle function templates that behave as casts, such as
+``llvm::dyn_cast``, ``boost::lexical_cast`` or ``gsl::narrow_cast``.
+
----------------
aaron.ballman wrote:
> Should this be moved into Known Limitations?
OK.


https://reviews.llvm.org/D25316