[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 inline comments.


================
Comment at: test/Analysis/array-struct-region.cpp:31
+  bool check() const { return this == this + 0; }
+  bool operator !() const { return this != this + 0; }
 
----------------
NoQ wrote:
> rtrieu wrote:
> > 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.
> I'd rather disable the newly introduced warning on this test file, because this is a path-sensitive static analysis test and this change may accidentally ruin the original test.
Makes sense.  The warning is now disabled via -Wno flags on the RUN lines.


================
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}}
----------------
rtrieu wrote:
> 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.
A little recursive checking and now we check array accesses.


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


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

https://reviews.llvm.org/D66045