[PATCH] D66045: Improve detection of same value in comparisons

[Richard Trieu via Phabricator via cfe-commits]

rtrieu marked 3 inline comments as done.
rtrieu added a comment.

In D66045#1624389 <https://reviews.llvm.org/D66045#1624389>, @jfb wrote:

> Does this work for unions as well?


This will work for union accesses via the same member name, but not different member names.

  union U { int x; int y; } u;
  bool b1 = u.x == u.x;  // warn always true
  bool b2 = u.x == u.y;  // no warning

It's easy to do when the types are the same, but more difficult when the types are different, so keeping with the same name member access is the safe way for now.



================
Comment at: test/Analysis/array-struct-region.cpp:31
+  bool check() const { return this == this + 0; }
+  bool operator !() const { return this != this + 0; }
 
----------------
jfb wrote:
> Is this the only way? Or do casts also disable the diagnostic?
There's other ways to do this.  I picked "+ 0" here.  Explicit casts, like to void* or const S* would also disable it.  I think pragmas would also work.  I don't particularly care which way since this is an analyzer test.


================
Comment at: test/SemaCXX/self-comparison.cpp:78
+  return S::static_field == s1.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array == s1.array;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.s.static_field == S::static_field;  // expected-warning {{self-comparison always evaluates to true}}
----------------
jfb wrote:
> `s1.array[0] == s1.array[0]`?
No, array accesses aren't checked yet.  But it's a good idea to look into it.


================
Comment at: test/SemaCXX/self-comparison.cpp:87
+};
+} // namespace member_tests
----------------
jfb wrote:
> The test only has `==`. Do other operators trigger?
All the standard comparison operators will work here.  I'll add tests.


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

https://reviews.llvm.org/D66045