[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

davrec added a comment. > There are already two way more sophisticated (forgive me my bias) > implementations in Clang that are for checking if two statements or decls are > the same. > > 1. ODRHash, used in modules to discover ODR violations > 2. ASTStructuralEquivalenceContext, used in

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

martong added a comment. In D114622#3200678 , @NoQ wrote: > These checks are almost 2000 lines of code each and it looks like all they do > is figure out if two statements are the same and we have a very flexible > reusable solution for this sort of

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

davrec added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp:372 +} + /// Determines whether two statement trees are identical regarding davrec wrote: > I would strongly support turning these functions into methods of

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

davrec added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:59 + RTy->getCanonicalTypeUnqualified(); + + // FIXME: We should probably check the other kinds of nested name specifiers. steakhal wrote: >

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal marked an inline comment as done. steakhal added a comment. In D114622#3200678 , @NoQ wrote: > These checks are almost 2000 lines of code each and it looks like all they do > is figure out if two statements are the same and we have a very

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

NoQ added a comment. P.S. `bugprone-branch-clone` seems to have attempted to use `CloneDetector` in the past but now it's no more than a dead #include. I wonder what happened there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114622/new/ https://reviews.llvm.org/D114622

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

NoQ added a comment. These checks are almost 2000 lines of code each and it looks like all they do is figure out if two statements are the same and we have a very flexible reusable solution for this sort of stuff - `CloneDetector`, but none of them use it. Your patch demonstrates that they all

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal updated this revision to Diff 395150. steakhal added a comment. Sorry for the late update, but here we are: Now, I properly handle all kinds of `NestedNameSpecifiers`. Basically, I verify if the decls are matching, then I use the nested name specifiers for finding mismatches. I look

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal added a comment. Sorry for my late response, I'm busy with other things right now. I plan to come back to this in a couple of weeks. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:52 static bool areEquivalentNameSpecifier(const

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

davrec added a comment. A couple thoughts/cases to consider... Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:52 static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left, const

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:54-55 const NestedNameSpecifier *Right) { - llvm::FoldingSetNodeID LeftID, RightID; - Left->Profile(LeftID); -

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal updated this revision to Diff 390032. steakhal marked 3 inline comments as done. steakhal added a comment. Fixing nits: renaming variables, etc. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114622/new/ https://reviews.llvm.org/D114622 Files:

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal updated this revision to Diff 390023. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114622/new/ https://reviews.llvm.org/D114622 Files: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal added a comment. In D114622#3155605 , @whisperity wrote: > I haven't looked at the patch in detail yet, but I know I've run into the > issue this aims to fix when developing a Tidy check! > > There is a `//

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:54-55 const NestedNameSpecifier *Right) { - llvm::FoldingSetNodeID LeftID, RightID; - Left->Profile(LeftID); -

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

whisperity added a comment. I haven't looked at the patch in detail yet, but I know I've run into the issue this aims to fix when developing a Tidy check! There is a `// NOLINTNEXTLINE(misc-redundant-expression)` directive in the source file

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal created this revision. steakhal added reviewers: aaron.ballman, alexfh, hokein, courbet, NoQ, xazax.hun, martong, whisperity, davrec. Herald added subscribers: carlosgalvezp, manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet,