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

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.


================
Comment at: test/SemaCXX/self-comparison.cpp:87
+};
+} // namespace member_tests
----------------
rtrieu wrote:
> jfb wrote:
> > rtrieu wrote:
> > > 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.
> > `operator<=>`?
> > 
> > Is there a separate test for user-defined operators as well? What makes sense in these cases? Technically a user-defined comparison could do anything... but I think this warning makes sense for them as well.
> operator<=> test was added to test/SemaCXX/compare-cxx2a.cpp because the operator requires header that this test doesn't include.
> 
> The warning currently doesn't check user-defined operators and I've include a test below to show that.  If a warning for user-defined operators was created, it would not belong in -Wtautological-compare because we would not know the actual result.  The best we could do is to say there was a self compare with a user-defined operator.  It would also need to hook into the AST at a different point.  Builtin compare operators are checked in BinaryOperator nodes.  User-defined operators would need to be checked at CallExpr or CXXOperatorCallExpr instead.  This is different enough that it should be in a different patch.
Thanks, this looks great!


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

https://reviews.llvm.org/D66045