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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
