new loplugin: pointerbool

look for possibly bogus implicit conversions to bool when passing
(normally pointer) args to bool params.

this plugin comes in the wake of a couple of bugs caused by refactoring,
where some of the call sites were not currently updated.

Of the changes, the following are real bugs:

desktop/../dp_persmap.cxx

StartInputFieldDlg
    in sw/../fldmgr.cxx
which occurred as a result of
    commit 39d719a80d8c87856c84e3ecd569d45fa6f8a30e
    Date:   Tue May 3 11:39:37 2016 +0200
    tdf#99529 sw: don't pop up input field dialog before inserting field

CSerializationURLEncoded::encode_and_append in
    forms/../serialization_urlencoded.cxx

XclExpCFImpl::XclExpCFImpl
    in sc/../xecontent.cxx
I have no idea how to properly fix this, just made a guess.

SwDocTest::test64kPageDescs
    in sw/qa/core/uwriter.cxx
which looks like a simple copy/paste error.

Change-Id: I795ebd5ef485a1d36863dc27fe13832989f5a441
Reviewed-on: https://gerrit.libreoffice.org/48291
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>

4 files changed, 167 insertions, 0 deletions

diff --git a/compilerplugins/clang/check.cxx b/compilerplugins/clang/check.cxx
index 2a7b0a978348..8b3e409be2d1 100644
--- a/compilerplugins/clang/check.cxx
+++ b/compilerplugins/clang/check.cxx
@@ -138,6 +138,15 @@ TypeCheck TypeCheck::NotSubstTemplateTypeParmType() const {
         ? *this : TypeCheck();
 }
 
+TypeCheck TypeCheck::SubstTemplateTypeParmType() const {
+    if (!type_.isNull()) {
+        if (auto const t = type_->getAs<clang::SubstTemplateTypeParmType>()) {
+            return TypeCheck(t->desugar());
+        }
+    }
+    return TypeCheck();
+}
+
 ContextCheck DeclCheck::Operator(clang::OverloadedOperatorKind op) const {
     assert(op != clang::OO_None);
     auto f = llvm::dyn_cast_or_null<clang::FunctionDecl>(decl_);
diff --git a/compilerplugins/clang/check.hxx b/compilerplugins/clang/check.hxx
index af6e8263df39..f31599f1b153 100644
--- a/compilerplugins/clang/check.hxx
+++ b/compilerplugins/clang/check.hxx
@@ -72,6 +72,8 @@ public:
 
     TypeCheck NotSubstTemplateTypeParmType() const;
 
+    TypeCheck SubstTemplateTypeParmType() const;
+
 private:
     TypeCheck() = default;
 
diff --git a/compilerplugins/clang/pointerbool.cxx b/compilerplugins/clang/pointerbool.cxx
new file mode 100644
index 000000000000..543e8c6533c8
--- /dev/null
+++ b/compilerplugins/clang/pointerbool.cxx
@@ -0,0 +1,124 @@
+/* -*- 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"
+
+/**
+  Look for calls where the param is bool but the call-site-arg is pointer.
+*/
+
+namespace
+{
+class PointerBool : public RecursiveASTVisitor<PointerBool>, public loplugin::Plugin
+{
+public:
+    explicit PointerBool(loplugin::InstantiationData const& data)
+        : Plugin(data)
+    {
+    }
+
+    virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    bool VisitCallExpr(CallExpr const*);
+
+private:
+    llvm::Optional<APSInt> getCallValue(const Expr* arg);
+};
+
+bool PointerBool::VisitCallExpr(CallExpr const* callExpr)
+{
+    if (ignoreLocation(callExpr))
+        return true;
+    // TODO this doesn't currently work, the args and the params don't seem to line up
+    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,
+               "possibly unwanted implicit conversion when calling bool param", arg->getExprLoc())
+            << arg->getSourceRange();
+        report(DiagnosticsEngine::Note, "method here", param->getLocation())
+            << param->getSourceRange();
+        arg->getType()->dump();
+    }
+    return true;
+}
+
+llvm::Optional<APSInt> PointerBool::getCallValue(const Expr* arg)
+{
+    arg = arg->IgnoreParenCasts();
+    if (auto defArg = dyn_cast<CXXDefaultArgExpr>(arg))
+    {
+        arg = defArg->getExpr()->IgnoreParenCasts();
+    }
+    // ignore this, it seems to trigger an infinite recursion
+    if (isa<UnaryExprOrTypeTraitExpr>(arg))
+    {
+        return llvm::Optional<APSInt>();
+    }
+    APSInt x1;
+    if (arg->EvaluateAsInt(x1, compiler.getASTContext()))
+    {
+        return x1;
+    }
+    return llvm::Optional<APSInt>();
+}
+
+loplugin::Plugin::Registration<PointerBool> X("pointerbool", true);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/pointerbool.cxx b/compilerplugins/clang/test/pointerbool.cxx
new file mode 100644
index 000000000000..276a95ae1e00
--- /dev/null
+++ b/compilerplugins/clang/test/pointerbool.cxx
@@ -0,0 +1,32 @@
+/* -*- 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 <sal/types.h>
+#include <com/sun/star/uno/Sequence.hxx>
+
+#define FALSE 0
+
+void func_ptr(int*);
+void func_bool(bool); // expected-note {{method here [loplugin:pointerbool]}}
+void func_salbool(sal_Bool);
+
+void test1(int* p1)
+{
+    func_ptr(p1);
+    func_bool(
+        p1); // expected-error {{possibly unwanted implicit conversion when calling bool param [loplugin:pointerbool]}}
+    // no warning expected
+    func_bool(FALSE);
+    func_salbool(sal_False);
+    func_salbool(sal_True);
+    css::uno::Sequence<sal_Bool> aSeq;
+    func_bool(aSeq[0]);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */