donat.nagy marked 3 inline comments as done. donat.nagy added a comment. **AstDiff:**
I looked at the AstDiff library, but I think that it is overkill for my goals. The main feat of the AstDiff library is analyzing and presenting the differences between two AST subtrees, while this checker only needs a simple yes/no answer for "Are these branches identical?". **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. I think that in some situations this would be useful for detecting redundant or incorrect parts of macro-infected code. ================ Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:31 +/// an if/else if/else chain is one statement (which may be a CompoundStmt). +using SwitchBranch = llvm::SmallVector<const Stmt *, 2>; +} // anonymous namespace ---------------- JonasToth wrote: > maybe plural for the typename would fit better, as its a vector of multiple > elements? This type represents one branch in a switch statement (which consists of multiple statements). I cannot think of a descriptive, short name which also happens to be plural and I do not want to use a monstrosity like `StatementsInSwitchBranch`. By the way, can anyone think of shorter, but still clear names for `areStatementsIdentical()` and `areSwitchBranchesIdentical()` ? ================ Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:187 + while (FamilyBegin < End) { + auto FamilyEnd = FamilyBegin + 1; + while (FamilyEnd < End && ---------------- JonasToth wrote: > i think the name `FamilyEnd` is not very descriptive, maybe > `SubsequentBranch` or so? I have renamed FamilyBegin/FamilyEnd to BeginCurrent/EndCurrent. I kept `begin` and `end` in the names to emphasize that this is is an iterator range (and altered the comment to include the expression "iterator range"). Is this naming choice clear enough? ================ Comment at: test/clang-tidy/bugprone-branch-clone.cpp:200 + +void test_macro1(int in, int &out) { + PASTE_CODE( ---------------- The tests for macro handling start here. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.llvm.org/D54757 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits