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

[Balázs Benics via Phabricator via cfe-commits]

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, baloghadamsoftware, dylanmckay.
Herald added a reviewer: Szelethus.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Consider this example:

  #include <type_traits>
  using std::is_same;
  bool top() {
    return is_same<char, int>::value || is_same<char, long>::value;
  }

We can see that the `value` static members are different, but in fact,
they refer to the same VarDecl entity. It is because both `is_same`
class instances inherit from the common `false_type` class, which
actually owns the `value` member.

The `alpha.core.IdenticalExpr` Static Analyzer checker actually warns
for this, since it only checked if the referred `Decls` are the same.
Interestingly, the clang-tidy `misc-redundant-expression` also reports
this is for the exact same reason, so I'm fixing both of them at the same
time.

This patch fixes this by checking if the `Decls` are the same and if they
are it will try to acquire the nested namespace specifier as a type and
compare them as well. If they are inequal, that means that the
//spelling// of the nested namespace specifiers actually differed, this
it's likely to be a false-positive.

I'd like to note that the checker/check was actually right about that
the expressions were semantically equal in that given context, however,
we still don't want these reports in general.

We could introduce checker options to finetune this behavior if needed.

PS: According to the code coverage of the test, all touched parts are
completely covered.

Thanks David Rector (@davrec) for the idea on the cfe-dev forum:
https://lists.llvm.org/pipermail/cfe-dev/2021-November/069389.html


https://reviews.llvm.org/D114622

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/test/Analysis/identical-expressions.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114622.389965.patch
Type: text/x-patch
Size: 8226 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211126/54120474/attachment-0001.bin>