[Libreoffice-commits] core.git: compilerplugins/clang

[Stephan Bergmann (via logerrit)]

 compilerplugins/clang/cppunitassertequals.cxx      |   14 ++++++++++----
 compilerplugins/clang/test/cppunitassertequals.cxx |   11 ++++++++++-
 compilerplugins/clang/test/cppunitassertequals.hxx |    5 ++++-
 3 files changed, 24 insertions(+), 6 deletions(-)

New commits:
commit 3d6214624128626c056b0a15ff6d654f57f149d9
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Apr 28 10:16:56 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Apr 28 13:39:01 2020 +0200

    Silence loplugin:cppunitassertequals when comparing pointer against nullptr
    
    I need that for another upcoming commit.
    
    Change-Id: If7e567c731e454070bf8ad9efb5c2f28ff9049e6
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93031
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/cppunitassertequals.cxx b/compilerplugins/clang/cppunitassertequals.cxx
index 930ecdd74e13..3647540baf8a 100644
--- a/compilerplugins/clang/cppunitassertequals.cxx
+++ b/compilerplugins/clang/cppunitassertequals.cxx
@@ -46,7 +46,8 @@ private:
     void checkExpr(
         SourceRange range, StringRef name, Expr const * expr, bool negated);
 
-    void reportEquals(SourceRange range, StringRef name, bool negative);
+    void reportEquals(
+        SourceRange range, StringRef name, bool negative, Expr const * lhs, Expr const * rhs);
 
     bool isCompileTimeConstant(Expr const * expr);
 };
@@ -187,7 +188,7 @@ void CppunitAssertEquals::checkExpr(
     if (auto const e = dyn_cast<BinaryOperator>(expr)) {
         auto const op = e->getOpcode();
         if ((!negated && op == BO_EQ) || (negated && op == BO_NE)) {
-            reportEquals(range, name, op == BO_NE);
+            reportEquals(range, name, op == BO_NE, e->getLHS(), e->getRHS());
             return;
         }
 #if 0 // TODO: enable later
@@ -206,7 +207,7 @@ void CppunitAssertEquals::checkExpr(
         if ((!negated && op == OO_EqualEqual)
             || (negated && op == OO_ExclaimEqual))
         {
-            reportEquals(range, name, op == OO_ExclaimEqual);
+            reportEquals(range, name, op == OO_ExclaimEqual, e->getArg(0), e->getArg(1));
             return;
         }
         return;
@@ -214,8 +215,13 @@ void CppunitAssertEquals::checkExpr(
 }
 
 void CppunitAssertEquals::reportEquals(
-    SourceRange range, StringRef name, bool negative)
+    SourceRange range, StringRef name, bool negative, Expr const * lhs, Expr const * rhs)
 {
+    if (lhs->IgnoreImpCasts()->getType()->isNullPtrType()
+        != rhs->IgnoreImpCasts()->getType()->isNullPtrType())
+    {
+        return;
+    }
     report(
         DiagnosticsEngine::Warning,
         ("rather call"
diff --git a/compilerplugins/clang/test/cppunitassertequals.cxx b/compilerplugins/clang/test/cppunitassertequals.cxx
index 9448ddc02950..d0825000be07 100644
--- a/compilerplugins/clang/test/cppunitassertequals.cxx
+++ b/compilerplugins/clang/test/cppunitassertequals.cxx
@@ -19,7 +19,9 @@
 #define TEST1 CPPUNIT_ASSERT(b1 == b2)
 #define TEST2(x) x
 
-void test(bool b1, bool b2, OUString const & s1, OUString const & s2, T t) {
+void test(
+    bool b1, bool b2, OUString const & s1, OUString const & s2, T t, void * p, std::nullptr_t n)
+{
     CppUnit::Asserter::failIf(b1,"");
 #if 0 // TODO: enable later
     CPPUNIT_ASSERT(b1 && b2); // expected-error {{rather split into two CPPUNIT_ASSERT [loplugin:cppunitassertequals]}}
@@ -50,6 +52,13 @@ void test(bool b1, bool b2, OUString const & s1, OUString const & s2, T t) {
     CPPUNIT_ASSERT(operator ==(s1, s1));
     CPPUNIT_ASSERT(t.operator ==(t));
 
+    // `P == nullptr` for P of pointer type is awkward to write with CPPUNIT_ASSERT_EQUAL, and the
+    // actual pointer values that would be printed if CPPUNIT_ASSERT_EQUAL failed would likey not be
+    // very meaningful, so let it use CPPUNIT_ASSERT (but stick to CPPUNIT_ASSERT_EQUAL for
+    // consistency in the unlikely case that P is of type std::nullptr_t):
+    CPPUNIT_ASSERT(p == nullptr);
+    CPPUNIT_ASSERT(n == nullptr); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}}
+
     // There might even be good reasons(?) not to warn inside explicit casts:
     CPPUNIT_ASSERT(bool(b1 && b2));
     CPPUNIT_ASSERT(bool(b1 == b2));
diff --git a/compilerplugins/clang/test/cppunitassertequals.hxx b/compilerplugins/clang/test/cppunitassertequals.hxx
index b844c8387e67..7a14886ad9eb 100644
--- a/compilerplugins/clang/test/cppunitassertequals.hxx
+++ b/compilerplugins/clang/test/cppunitassertequals.hxx
@@ -12,11 +12,14 @@
 
 #include "sal/config.h"
 
+#include <cstddef>
+
 #include "rtl/ustring.hxx"
 
 struct T { bool operator ==(T); };
 
-void test(bool b1, bool b2, OUString const & s1, OUString const & s2, T t);
+void test(
+    bool b1, bool b2, OUString const & s1, OUString const & s2, T t, void * p, std::nullptr_t n);
 
 #endif