njames93 added a comment.
Thanks for working on this.
There seems to be many places in this check where names are confusing, `Var`
referring to a `MemberExpr`, `DeclRef` refering to a `MemberExpr`. I'd prefer
if the names that you have added were renamed to something more in line with
what they represent.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp:82
+
+ if (const auto *Var = Result.Nodes.getNodeAs<MemberExpr>("expr")) {
+ const Decl *D = Var->getMemberDecl();
----------------
This should be an `else if` or the previous `if` should return. Saves redundant
checking for `MemberExpr` when `DeclRefExpr` already matched.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp:108-126
+ if (d.b) {
+ *d.b = true; // no-warning
+ }
+
+ if (d.b) {
+ d.b[0] = false; // no-warning
+ }
----------------
nit: You don't need to recycle all the test cases, the logic for that is
already checked above, just a few showing member expr being detected properly
will suffice.
Don't have huge preference on this so feel free to ignore.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83188/new/
https://reviews.llvm.org/D83188
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
