donat.nagy added a comment.
Mostly LGTM, I suggested two cosmetic changes.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:55
+ bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+ // Ignore lambdas
+ return true;
----------------
Bikeshedding: these comments are very useful, but put them after the return
statements. The //(meaningful content) / (number of lines)// ratio is already
abysmal in visitor classes like this, let's try to avoid what we can.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:84
+ for (const Attr *A : S->getAttrs()) {
+ if (FallThroughAttr::classof(A))
+ return false;
----------------
If I understand it correctly, `Class::classof(Pointer)` is an implementation
detail of the more flexible and slightly magical `isa<Class>(Obj)` utility. The
clang-tidy codebase contains lots of `isa<>` and only a single direct reference
to `classof()` so I'd suggest using `isa<>` here for the sake of consistency.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147889/new/
https://reviews.llvm.org/D147889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
