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

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

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

> **Macros:**
>
> The current implementation of the check only looks at the preprocessed code, therefore it detects the situations where the duplication is created by macros. I added some tests to highlight this behavior.


The only option I see to detect macro related errors is to use a `Lexer`, and compare the tokens one by one, but that might be overkill, and I'm aware of a check in the works that already implements that. I think it's perfectly fine not to cover those cases, but you should definitely document it somewhere, preferably in the rst file.

> I think that in some situations this would be useful for detecting redundant or incorrect parts of macro-infected code.

How about this?

  #define PANDA_CUTE_FACTOR 9001
  #define CAT_CUTE_FACTOR 9001
  
  if (whatever())
    return PANDA_CUTE_FACTOR;
  else
    return CAT_CUTE_FACTOR;

Your check would warn here, but it is possible, if not likely that the definition of those macros will eventually change. Again, it's fine not to cover this, but this //is// a false positive in a sense.

I have to agree with the folks before me, this patch is amazing, great job!



================
Comment at: test/clang-tidy/bugprone-branch-clone.cpp:200
+
+void test_macro1(int in, int &out) {
+  PASTE_CODE(
----------------
donat.nagy wrote:
> The tests for macro handling start here.
Maybe add dividers? :) The `//===--------------===//` thing, you know.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54757