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

[Whisperity via Phabricator via cfe-commits]

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);
-  Right->Profile(RightID);
-  return LeftID == RightID;
+  if (const auto *L = Left->getAsType())
+    if (const auto *R = Right->getAsType())
+      return L->getCanonicalTypeUnqualified() ==
----------------
(Wrt. to the comment about not typing stuff out directly, I guess `L` and `R` here will be `X<char>` and `X<long>` and thus not the same.)


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:54-57
+  if (const auto *L = Left->getAsType())
+    if (const auto *R = Right->getAsType())
+      return L->getCanonicalTypeUnqualified() ==
+             R->getCanonicalTypeUnqualified();
----------------
whisperity wrote:
> (Wrt. to the comment about not typing stuff out directly, I guess `L` and `R` here will be `X<char>` and `X<long>` and thus not the same.)
(Style nit to lessen the indent depth, until C++17...)


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:132-134
+    const DeclRefExpr *DeclRef1 = cast<DeclRefExpr>(Left);
+    const DeclRefExpr *DeclRef2 = cast<DeclRefExpr>(Right);
+    return areEquivalentDeclRefs(DeclRef1, DeclRef2);
----------------
(Style nit. Or `LeftDR`, `RightDR`.)


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:822
+
+  using my_char = char;
+  sink |= my_trait<char>::value || my_trait<my_char>::value;
----------------
Are there any more ways we could trick this check to cause false positives? Something through which it doesn't look the same but still refers to the same thing?

At the top of my head I'm thinking about something along the lines of this:

```lang=cpp
template <typename T>
struct X {
  template <typename U>
  static constexpr bool same = std::is_same_v<T, U>;
};

X<char> ch() { return X<char>{}; }
X<int> i() { return X<int>{}; }

void test() {
  coin ? ch().same<long> : i().same<long>;
}
```

So the "type name" where we want to look up isn't typed out directly.
Or would these still be caught (and thus fixed) by looking at a `NestedNameSpecifier`?


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

https://reviews.llvm.org/D114622