[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy marked 15 inline comments as done.
donat.nagy added a comment.

I will add tests for macro handling later.



================
Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:71
+    // Check whether this `if` is part of an `else if`:
+    if (const auto *IP =
+            dyn_cast<IfStmt>(Result.Nodes.getNodeAs<Stmt>("ifParent"))) {
----------------
JonasToth wrote:
> This if should always be true, no? The matcher will always bind `ifParent`
I moved this check to the AST matcher.


================
Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:116
+        Branches.push_back(Else);
+        // Note: We will exit the while loop here.
+      }
----------------
JonasToth wrote:
> This note is misplaced? The exit is at the `break`.
The note was placed correctly, but I replaced it with a break.


================
Comment at: docs/clang-tidy/checks/list.rst:13
    abseil-str-cat-append
+   abseil-string-find-startswith
    android-cloexec-accept
----------------
JonasToth wrote:
> spurious change
It was added automatically by the python script, which sorts this list alphabetically. If I remove it now, it will be automatically re-added later.


================
Comment at: test/clang-tidy/bugprone-branch-clone.cpp:4
+void test_basic1(int in, int &out) {
+  if (in > 77)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone]
----------------
JonasToth wrote:
> could you please add tests for ternary operators?
Currently the check does not handle ternary operators, but I added some checks reflecting this.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54757/new/

https://reviews.llvm.org/D54757