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

Mon Dec 20 02:47:00 PST 2021

steakhal marked an inline comment as done.
steakhal added a comment.

In D114622#3200678 <https://reviews.llvm.org/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 stuff - `CloneDetector`, but none of them use it. Your patch demonstrates that they all have the same bugs that need to be fixed in each of them separately, so reusability seems really valuable. If I was to work on these checks, the first thing I'd try would be to throw out their machines and plug in a reusable solution.

Well, yes. Ideally, we should remove probably the ClangSA implementation, since it does basically the same thing. Right now I'm focusing on fixing this FP, and I don't really want to tip my toe into anything bigger than that.

About the `CloneDetector`, well in February this year we observed that it basically halted an analysis, due to some unfortunate situation. I saw `areSequencesClones()` stringifying QualTypes for hours if not days.
We had other priorities to focus, thus we could not get there reproducing and fixing the issue with that, but that could be a potential reason why people don't want to use that. We should really dig into that at some point.
For the record, at the time it was analyzing llvm itself, the `AMDGPUAsmParser.cpp` file to be precise.

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

https://reviews.llvm.org/D114622