[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

This revision was automatically updated to reflect the committed changes. Closed by commit rL359142: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals (authored by dhinton, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda added a comment. In D59802#1475596 , @aaron.ballman wrote: > In D59802#1474300 , @hintonda wrote: > > > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, > > and it seems to

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added a comment. In D59802#1474300 , @hintonda wrote: > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, > and it seems to work fine. However, it did miss this one: > > - if (V && isa(V) && (EntInst = cast(V)) &&

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda added a comment. @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, and it seems to work fine. However, it did miss this one: - if (V && isa(V) && (EntInst = cast(V)) && +if (isa_and_nonnull(V) && (EntInst = cast(V)) && It got the first, but not the

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added a comment. In D59802#1467430 , @hintonda wrote: > In D59802#1467363 , @aaron.ballman > wrote: > > > LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. > > > Thanks.

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 195270. hintonda added a comment. - Alphabetize registration calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files:

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added a comment. In D59802#1467363 , @aaron.ballman wrote: > LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. Thanks. I'll ping the list again at the end of the week, then

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 194782. hintonda added a comment. - Update isa_and_nonnull diagnostic message, and fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files:

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:37-39 +/// if (var && isa(var)) {} +/// // is replaced by: +/// if (dyn_cast_or_null(var.foo())) {}

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp:125 +diag(MatchedDecl->getBeginLoc(), + "isa_and_nonnull<> is cleaner and more efficient") +<<

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 194017. hintonda added a comment. Explicitly check for implicitCastExpr and add additional negative matching test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 193968. hintonda added a comment. - Use new isa_and_nonnull, and cleanup code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files:

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers hintonda wrote: > aaron.ballman wrote: >

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<<

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<<

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers hintonda wrote: > @aaron.ballman: This

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 193379. hintonda added a comment. - Remove spaces in docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-isa-or-dyn-cast-in-conditionals.rst:6 + + Looks at conditionals and finds cases of ``cast<>``, which will + assert rather than return a null pointer, and ``dyn_cast<>``