loplugin:simplifypointertobool improve

to look for the
x.get() != null
pattern, which can be simplified to
x

I'll do the
x.get() == nullptr
pattern in a separate patch, to reduce the chances of a mistake

Change-Id: I45e0d178e75359857cdf50d712039cb526016555
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95354
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 73 insertions, 1 deletions

diff --git a/compilerplugins/clang/simplifypointertobool.cxx b/compilerplugins/clang/simplifypointertobool.cxx
index 46a80a770757..4f8abb52c74f 100644
--- a/compilerplugins/clang/simplifypointertobool.cxx
+++ b/compilerplugins/clang/simplifypointertobool.cxx
@@ -45,6 +45,7 @@ public:
     }
 
     bool VisitImplicitCastExpr(ImplicitCastExpr const*);
+    bool VisitBinaryOperator(BinaryOperator const*);
 
     bool PreTraverseUnaryLNot(UnaryOperator* expr)
     {
@@ -303,7 +304,8 @@ bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castEx
         return true;
     if (castExpr->getCastKind() != CK_PointerToBoolean)
         return true;
-    auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr()->IgnoreParens());
+    auto memberCallExpr
+        = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr()->IgnoreParenImpCasts());
     if (!memberCallExpr)
         return true;
     auto methodDecl = memberCallExpr->getMethodDecl();
@@ -416,6 +418,37 @@ bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castEx
     return true;
 }
 
+bool SimplifyPointerToBool::VisitBinaryOperator(BinaryOperator const* binOp)
+{
+    if (ignoreLocation(binOp))
+        return true;
+    auto opCode = binOp->getOpcode();
+    //TODO    if (opCode != BO_EQ && opCode != BO_NE)
+    //        return true;
+    if (opCode != BO_NE)
+        return true;
+    const Expr* possibleMemberCall = nullptr;
+    if (isa<CXXNullPtrLiteralExpr>(binOp->getLHS()->IgnoreParenImpCasts()))
+        possibleMemberCall = binOp->getRHS();
+    else if (isa<CXXNullPtrLiteralExpr>(binOp->getRHS()->IgnoreParenImpCasts()))
+        possibleMemberCall = binOp->getLHS();
+    else
+        return true;
+    auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(possibleMemberCall);
+    if (!memberCallExpr)
+        return true;
+    auto methodDecl = memberCallExpr->getMethodDecl();
+    if (!methodDecl || !methodDecl->getIdentifier() || methodDecl->getName() != "get")
+        return true;
+    if (!loplugin::isSmartPointerType(memberCallExpr->getImplicitObjectArgument()))
+        return true;
+    report(DiagnosticsEngine::Warning,
+           std::string("simplify, convert to ") + (opCode == BO_EQ ? "'!x'" : "'x'"),
+           binOp->getExprLoc())
+        << binOp->getSourceRange();
+    return true;
+}
+
 loplugin::Plugin::Registration<SimplifyPointerToBool> simplifypointertobool("simplifypointertobool",
                                                                             true);
 
diff --git a/compilerplugins/clang/test/simplifypointertobool.cxx b/compilerplugins/clang/test/simplifypointertobool.cxx
index e4bf14c40f45..05f78d52ed78 100644
--- a/compilerplugins/clang/test/simplifypointertobool.cxx
+++ b/compilerplugins/clang/test/simplifypointertobool.cxx
@@ -8,6 +8,7 @@
  */
 
 #include <memory>
+#include "com/sun/star/uno/XInterface.hpp"
 
 void foo();
 
@@ -30,6 +31,44 @@ void test2(std::shared_ptr<int> p)
     // expected-error@+1 {{simplify, drop the get() [loplugin:simplifypointertobool]}}
     if (p.get())
         foo();
+    // TODOexpected-error@+1 {{simplify, convert to '!x' [loplugin:simplifypointertobool]}}
+    if (p.get() == nullptr)
+        foo();
+    // TODOexpected-error@+1 {{simplify, convert to '!x' [loplugin:simplifypointertobool]}}
+    if (p == nullptr)
+        foo();
+    // TODOexpected-error@+1 {{simplify, convert to 'x' [loplugin:simplifypointertobool]}}
+    if (p != nullptr)
+        foo();
+    // TODOexpected-error@+1 {{simplify, convert to '!x' [loplugin:simplifypointertobool]}}
+    if (nullptr == p.get())
+        foo();
+    // expected-error@+1 {{simplify, convert to 'x' [loplugin:simplifypointertobool]}}
+    if (p.get() != nullptr)
+        foo();
+    // expected-error@+1 {{simplify, convert to 'x' [loplugin:simplifypointertobool]}}
+    if (nullptr != p.get())
+        foo();
+}
+
+void test2(int* p)
+{
+    // TODOexpected-error@+1 {{simplify, convert to '!x' [loplugin:simplifypointertobool]}}
+    if (p == nullptr)
+        foo();
+    // TODOexpected-error@+1 {{simplify, convert to 'x' [loplugin:simplifypointertobool]}}
+    if (p != nullptr)
+        foo();
+}
+
+void test2(css::uno::Reference<css::uno::XInterface> const& p)
+{
+    // expected-error@+1 {{simplify, drop the get() [loplugin:simplifypointertobool]}}
+    if (p.get())
+        foo();
+    // TODOexpected-error@+1 {{simplify, convert to '!x' [loplugin:simplifypointertobool]}}
+    if (p.get() == nullptr)
+        foo();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */