steakhal wrote: > I ran it on the Cppcheck codebase and there were several cases I would > consider false positives as well as some which are really awkward to fix (in > some cases leading to potentially unnecessary lookups). I will provide some > examples in the next few days.
I ran it on cppcheck, and I had 88 reports in total. See the results at [this](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Confirmed%20bug&review-status=False%20positive&detection-status=New&detection-status=Reopened&detection-status=Unresolved&run=steakhal-redundant-lookups-in-cppcheck&is-unique=off&diff-type=New) CodeChecker instance. Apply filters to look for only TPs or FPs. [Thanks for Ericsson hosting the instance btw] I looked each of the reports and classified them as "true positive" if I (likely) could refactor the code and would make sense doing so. FP otherwise if I wouldn't refactor the code. This gave me 56 TPs and 32 FPs. Note that sometimes the looked ugly, and had a lot of nesting. It may be possible to refactor but I'd decide not touching it. This is one form of a FP. The FP TP classification is of course arbitrary, and you may disagree with what I choose. These TPs are the cases where I think the code could be and should be refactored, nothing more. After seeing the results on cppcheck, I think it would make sense to use `ExprSequence` to ensure that the lookups are not on mutually exclusive paths for instance. It would also match better with the category of the check "performance". On the down side, it would make us no longer diagnose code like this: ```c++ if (cond) map[key] = foo(); else map[key] = bar(); ``` I've seen code like this part of the TP set. BTW I've not seen any misclassification of "map" or "set" objects due to the relaxed regex. https://github.com/llvm/llvm-project/pull/125420 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
