new loplugin:simplifypointertobool

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

5 files changed, 185 insertions, 37 deletions

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 14d8af8d65b3..ff15e06750e5 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -22,6 +22,7 @@
 
 #include "compat.hxx"
 #include "pluginhandler.hxx"
+#include "check.hxx"
 
 #if CLANG_VERSION >= 110000
 #include "clang/AST/ParentMapContext.h"
@@ -801,6 +802,43 @@ bool hasExternalLinkage(VarDecl const * decl) {
     return true;
 }
 
+bool isSmartPointerType(const Expr* e)
+{
+    // First check the object type as written, in case the get member function is
+    // declared at a base class of std::unique_ptr or std::shared_ptr:
+    auto const t = e->IgnoreImpCasts()->getType();
+    auto const tc1 = loplugin::TypeCheck(t);
+    if (tc1.ClassOrStruct("unique_ptr").StdNamespace()
+          || tc1.ClassOrStruct("shared_ptr").StdNamespace())
+        return true;
+    return isSmartPointerType(e->getType().getTypePtr());
+}
+
+bool isSmartPointerType(const clang::Type* t)
+{
+    // Then check the object type coerced to the type of the get member function, in
+    // case the type-as-written is derived from one of these types (tools::SvRef is
+    // final, but the rest are not; but note that this will fail when the type-as-
+    // written is derived from std::unique_ptr or std::shared_ptr for which the get
+    // member function is declared at a base class):
+    auto const tc2 = loplugin::TypeCheck(t);
+    if (tc2.ClassOrStruct("unique_ptr").StdNamespace()
+           || tc2.ClassOrStruct("shared_ptr").StdNamespace()
+           || tc2.Class("Reference").Namespace("uno").Namespace("star")
+                .Namespace("sun").Namespace("com").GlobalNamespace()
+           || tc2.Class("Reference").Namespace("rtl").GlobalNamespace()
+           || tc2.Class("SvRef").Namespace("tools").GlobalNamespace()
+           || tc2.Class("WeakReference").Namespace("tools").GlobalNamespace()
+           || tc2.Class("ScopedReadAccess").Namespace("Bitmap").GlobalNamespace()
+           || tc2.Class("ScopedVclPtrInstance").GlobalNamespace()
+           || tc2.Class("VclPtr").GlobalNamespace()
+           || tc2.Class("ScopedVclPtr").GlobalNamespace())
+    {
+        return true;
+    }
+    return false;
+}
+
 
 } // namespace
 
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 829d00518f50..bf238eb7767d 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -306,6 +306,9 @@ int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* ba
 // if it contained the 'extern' specifier:
 bool hasExternalLinkage(VarDecl const * decl);
 
+bool isSmartPointerType(const clang::Type*);
+bool isSmartPointerType(const Expr*);
+
 } // namespace
 
 #endif // COMPILEPLUGIN_H
diff --git a/compilerplugins/clang/redundantpointerops.cxx b/compilerplugins/clang/redundantpointerops.cxx
index 7012365aaef3..45b0783af0ab 100644
--- a/compilerplugins/clang/redundantpointerops.cxx
+++ b/compilerplugins/clang/redundantpointerops.cxx
@@ -53,8 +53,6 @@ public:
     bool VisitFunctionDecl(FunctionDecl const *);
     bool VisitMemberExpr(MemberExpr const *);
     bool VisitUnaryOperator(UnaryOperator const *);
-private:
-    bool isSmartPointerType(const Expr* e);
 };
 
 bool RedundantPointerOps::VisitFunctionDecl(FunctionDecl const * functionDecl)
@@ -104,7 +102,7 @@ bool RedundantPointerOps::VisitMemberExpr(MemberExpr const * memberExpr)
             if (methodDecl->getIdentifier() && methodDecl->getName() == "get")
             {
                 auto const e = cxxMemberCallExpr->getImplicitObjectArgument();
-                if (isSmartPointerType(e))
+                if (loplugin::isSmartPointerType(e))
                     report(
                         DiagnosticsEngine::Warning,
                         "'get()' followed by '->' operating on %0, just use '->'",
@@ -150,7 +148,7 @@ bool RedundantPointerOps::VisitUnaryOperator(UnaryOperator const * unaryOperator
         if (methodDecl->getIdentifier() && methodDecl->getName() == "get")
         {
             auto const e = cxxMemberCallExpr->getImplicitObjectArgument();
-            if (isSmartPointerType(e))
+            if (loplugin::isSmartPointerType(e))
                 report(
                     DiagnosticsEngine::Warning,
                     "'*' followed by '.get()' operating on %0, just use '*'",
@@ -162,39 +160,6 @@ bool RedundantPointerOps::VisitUnaryOperator(UnaryOperator const * unaryOperator
     return true;
 }
 
-bool RedundantPointerOps::isSmartPointerType(const Expr* e)
-{
-    // First check the object type as written, in case the get member function is
-    // declared at a base class of std::unique_ptr or std::shared_ptr:
-    auto const t = e->IgnoreImpCasts()->getType();
-    auto const tc1 = loplugin::TypeCheck(t);
-    if (tc1.ClassOrStruct("unique_ptr").StdNamespace()
-          || tc1.ClassOrStruct("shared_ptr").StdNamespace())
-        return true;
-
-    // Then check the object type coerced to the type of the get member function, in
-    // case the type-as-written is derived from one of these types (tools::SvRef is
-    // final, but the rest are not; but note that this will fail when the type-as-
-    // written is derived from std::unique_ptr or std::shared_ptr for which the get
-    // member function is declared at a base class):
-    auto const tc2 = loplugin::TypeCheck(e->getType());
-    if (tc2.ClassOrStruct("unique_ptr").StdNamespace()
-           || tc2.ClassOrStruct("shared_ptr").StdNamespace()
-           || tc2.Class("Reference").Namespace("uno").Namespace("star")
-                .Namespace("sun").Namespace("com").GlobalNamespace()
-           || tc2.Class("Reference").Namespace("rtl").GlobalNamespace()
-           || tc2.Class("SvRef").Namespace("tools").GlobalNamespace()
-           || tc2.Class("WeakReference").Namespace("tools").GlobalNamespace()
-           || tc2.Class("ScopedReadAccess").Namespace("Bitmap").GlobalNamespace()
-           || tc2.Class("ScopedVclPtrInstance").GlobalNamespace()
-           || tc2.Class("VclPtr").GlobalNamespace()
-           || tc2.Class("ScopedVclPtr").GlobalNamespace())
-    {
-        return true;
-    }
-    return false;
-}
-
 loplugin::Plugin::Registration< RedundantPointerOps > redundantpointerops("redundantpointerops");
 
 } // namespace
diff --git a/compilerplugins/clang/simplifypointertobool.cxx b/compilerplugins/clang/simplifypointertobool.cxx
new file mode 100644
index 000000000000..da6581915a1e
--- /dev/null
+++ b/compilerplugins/clang/simplifypointertobool.cxx
@@ -0,0 +1,121 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+
+#include <clang/AST/CXXInheritance.h>
+#include "plugin.hxx"
+#include "check.hxx"
+
+/**
+  Simplify boolean expressions involving smart pointers e.g.
+    if (x.get())
+  can be
+    if (x)
+*/
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+namespace
+{
+class SimplifyPointerToBool : public loplugin::FilteringPlugin<SimplifyPointerToBool>
+{
+public:
+    explicit SimplifyPointerToBool(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual void run() override
+    {
+        if (preRun())
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitImplicitCastExpr(ImplicitCastExpr const*);
+};
+
+bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castExpr)
+{
+    if (ignoreLocation(castExpr))
+        return true;
+    if (castExpr->getCastKind() != CK_PointerToBoolean)
+        return true;
+    auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr());
+    if (!memberCallExpr)
+        return true;
+    auto methodDecl = memberCallExpr->getMethodDecl();
+    if (!methodDecl || !methodDecl->getIdentifier() || methodDecl->getName() != "get")
+        return true;
+    //    castExpr->dump();
+    //    methodDecl->getParent()->getTypeForDecl()->dump();
+    if (!loplugin::isSmartPointerType(methodDecl->getParent()->getTypeForDecl()))
+        return true;
+    //    if (isa<CXXOperatorCallExpr>(callExpr))
+    //        return true;
+    //    const FunctionDecl* functionDecl;
+    //    if (isa<CXXMemberCallExpr>(callExpr))
+    //    {
+    //        functionDecl = dyn_cast<CXXMemberCallExpr>(callExpr)->getMethodDecl();
+    //    }
+    //    else
+    //    {
+    //        functionDecl = callExpr->getDirectCallee();
+    //    }
+    //    if (!functionDecl)
+    //        return true;
+    //
+    //    unsigned len = std::min(callExpr->getNumArgs(), functionDecl->getNumParams());
+    //    for (unsigned i = 0; i < len; ++i)
+    //    {
+    //        auto param = functionDecl->getParamDecl(i);
+    //        auto paramTC = loplugin::TypeCheck(param->getType());
+    //        if (!paramTC.AnyBoolean())
+    //            continue;
+    //        auto arg = callExpr->getArg(i)->IgnoreImpCasts();
+    //        auto argTC = loplugin::TypeCheck(arg->getType());
+    //        if (argTC.AnyBoolean())
+    //            continue;
+    //        // sal_Bool is sometimes disguised
+    //        if (isa<SubstTemplateTypeParmType>(arg->getType()))
+    //            if (arg->getType()->getUnqualifiedDesugaredType()->isSpecificBuiltinType(
+    //                    clang::BuiltinType::UChar))
+    //                continue;
+    //        if (arg->getType()->isDependentType())
+    //            continue;
+    //        if (arg->getType()->isIntegerType())
+    //        {
+    //            auto ret = getCallValue(arg);
+    //            if (ret.hasValue() && (ret.getValue() == 1 || ret.getValue() == 0))
+    //                continue;
+    //            // something like: priv->m_nLOKFeatures & LOK_FEATURE_DOCUMENT_PASSWORD
+    //            if (isa<BinaryOperator>(arg->IgnoreParenImpCasts()))
+    //                continue;
+    //            // something like: pbEmbolden ? FcTrue : FcFalse
+    //            if (isa<ConditionalOperator>(arg->IgnoreParenImpCasts()))
+    //                continue;
+    //        }
+    report(DiagnosticsEngine::Warning, "simplify, drop the get()", castExpr->getExprLoc())
+        << castExpr->getSourceRange();
+    //        report(DiagnosticsEngine::Note, "method here", param->getLocation())
+    //            << param->getSourceRange();
+    return true;
+}
+
+loplugin::Plugin::Registration<SimplifyPointerToBool>
+    simplifypointertobool("simplifypointertobool");
+
+} // namespace
+
+#endif // LO_CLANG_SHARED_PLUGINS
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/simplifypointertobool.cxx b/compilerplugins/clang/test/simplifypointertobool.cxx
new file mode 100644
index 000000000000..901262c3d474
--- /dev/null
+++ b/compilerplugins/clang/test/simplifypointertobool.cxx
@@ -0,0 +1,21 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <memory>
+
+void foo();
+
+void test1(std::unique_ptr<int> p2)
+{
+    // expected-error@+1 {{simplify, drop the get() [loplugin:simplifypointertobool]}}
+    if (p2.get())
+        foo();
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */