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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
