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

Richard Trieu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 15:17:17 PDT 2019

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.



More information about the cfe-commits mailing list