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

Mon Dec 3 08:41:30 PST 2018

JonasToth added a comment.

In D54757#1316899 <https://reviews.llvm.org/D54757#1316899>, @donat.nagy wrote:

> I applied this check to the llvm + clang codebase, and I was surprised to see that it produced about 600 results (for comparison, the clang-tidy checks which are enabled by default produce approximately 6000 results together). I didn't go through the whole list, but after examining a few dozen random reports it seems that most of these are true positives: relatively short branches (usually one or two lines of code) are repeated for no obvious reason. I would guess that most of these fall into the "correct but too verbose" case, because otherwise the tests would have caught the problem, but I didn't try to understand their context.
>
> I have seen a few false positives related to preprocessor trickery: when an `.inc` file is included to create a huge switch, sometimes it will become a huge switch with lots of identical branches. There were also some situations where the check reported identical branches which are annotated with different comments; I don't know if this should be considered a false positive.
>
> If this is acceptable, then I would be grateful if someone would commit this patch for me as I don't have commit rights.

Thank you!
Are the cases with `.inc` files easily silenceable with `// NOLINT` comments? In general code-generation through macros is not easily handled with clang-tidy, so all checks have the problem (e.g. GoogleTest is extremly chatty with everything), but would be interesting to know :)

All other cases sound like "a human should look and evaluate" and that's ok for clang-tidy as it doesn't claim finding ONLY bugs, but all kind of issues.

Thank you for the patch, if there are no objections by other reviewers I will commit tomorrow( ~24 hours from now)

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54757