aaron.ballman added a comment. In D59802#1443515 <https://reviews.llvm.org/D59802#1443515>, @hintonda wrote:
> In D59802#1443451 <https://reviews.llvm.org/D59802#1443451>, @hintonda wrote: > > > In D59802#1443340 <https://reviews.llvm.org/D59802#1443340>, @aaron.ballman > > wrote: > > > > > Should this check also try to find this pattern: > > > > > > if (dyn_cast<Frobble>(Bobble)) > > > ... > > > > > > > > > in order to recommend the proper replacement: > > > > > > if (isa<Frobble>(Bobble)) > > > ... > > > > > > > > > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it > > > would also cover this situation and I run into it during code reviews > > > with some frequency (more often than I run into `cast<>` being used in a > > > conditional). > > > > > > Yes, I can add that, and provide a fix-it too. Thanks... > > > I did a quick grep and found a few of these, but also found the `_or_null<>` > variety. Guess they'll need to stay the same since `isa<>` can't handle > nulls. Would it make sense to transform `if (dyn_cast_or_null<T>(Obj))` into `if (Obj && isa<T>(Obj))` or are there bad transformations from that? > Btw, I also found the same pattern used for `while()`, so I'll add that too. > Here's a sample of the patterns I'm seeing: > > ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213: > while (dyn_cast<NullStmt>(last_stmt)) { Hah, good catch! > ./clang/lib/CodeGen/CodeGenModule.cpp:1390: if > (dyn_cast_or_null<NamedDecl>(D)) . // <--- this one's okay I think this could be expressed as `if (D && isa<NamedDecl>(D))`, no? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
