New loplugin:consttobool

...to: "Find implicit conversions from non-'bool' constants (e.g., 'sal_False')
to 'bool'".

Due to how FALSE is defined as just

#define FALSE (0)

(i.e., a literal of type 'int') but TRUE is defined as

#define TRUE (!FALSE)

(i.e., an implicit conversion from 'int' to 'bool') in GLib (see the comment in
ConstToBool::VisitImplicitCastExpr), we get more warnings about uses of 'TRUE'
than of 'FALSE'.  For example, in libreofficekit/source/gtk/lokdocview.cxx there
is a warning about the 'TRUE' in

g_main_context_iteration(nullptr, TRUE);

but not about the 'FALSE' in

g_main_context_iteration(nullptr, FALSE);

(where the parameter of 'g_main_context_iteration' is of type 'gboolean').  Lets
live with that asymmetry for now...

(Besides the issues addressed directly in this commit, it also found the two
bogus asserts at 7e09d08807b5ba2fd8b9831557752a415bdad562 "Fix useless
assert(true) (which would never fire)" and
122a0be8ae480473bd1d7f35e197a2529f4621e3 "Fix useless assert(true) (which would
never fire)", plus 5f0d6df7f57ae281fe161e61c7f25d67453fddd2 "Use two-argument
form of static_assert".)

Change-Id: Id77322de9f94b85a7b65608a03e0e9865d14467b
Reviewed-on: https://gerrit.libreoffice.org/82667
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

2 files changed, 314 insertions, 0 deletions

diff --git a/compilerplugins/clang/consttobool.cxx b/compilerplugins/clang/consttobool.cxx
new file mode 100644
index 000000000000..4bd0d28e9eaa
--- /dev/null
+++ b/compilerplugins/clang/consttobool.cxx
@@ -0,0 +1,265 @@
+/* -*- 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/.
+ */
+
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+#include <cassert>
+#include <limits>
+#include <stack>
+
+#include "check.hxx"
+#include "plugin.hxx"
+
+// Find implicit conversions from non-'bool' constants (e.g., 'sal_False') to 'bool'.
+
+namespace
+{
+class ConstToBool final : public loplugin::FilteringPlugin<ConstToBool>
+{
+public:
+    explicit ConstToBool(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    bool PreTraverseLinkageSpecDecl(LinkageSpecDecl*)
+    {
+        assert(externCContexts_ != std::numeric_limits<unsigned int>::max()); //TODO
+        ++externCContexts_;
+        return true;
+    }
+
+    bool PostTraverseLinkageSpecDecl(LinkageSpecDecl*, bool)
+    {
+        assert(externCContexts_ != 0);
+        --externCContexts_;
+        return true;
+    }
+
+    bool TraverseLinkageSpecDecl(LinkageSpecDecl* decl)
+    {
+        bool ret = true;
+        if (PreTraverseLinkageSpecDecl(decl))
+        {
+            ret = FilteringPlugin::TraverseLinkageSpecDecl(decl);
+            PostTraverseLinkageSpecDecl(decl, ret);
+        }
+        return ret;
+    }
+
+    bool PreTraverseUnaryLNot(UnaryOperator* expr)
+    {
+        ignoredInAssert_.push(expr->getSubExpr());
+        return true;
+    }
+
+    bool PostTraverseUnaryLNot(UnaryOperator*, bool)
+    {
+        assert(!ignoredInAssert_.empty());
+        ignoredInAssert_.pop();
+        return true;
+    }
+
+    bool TraverseUnaryLNot(UnaryOperator* expr)
+    {
+        bool ret = true;
+        if (PreTraverseUnaryLNot(expr))
+        {
+            ret = FilteringPlugin::TraverseUnaryLNot(expr);
+            PostTraverseUnaryLNot(expr, ret);
+        }
+        return ret;
+    }
+
+    bool PreTraverseBinLAnd(BinaryOperator* expr)
+    {
+        ignoredInAssert_.push(expr->getRHS());
+        return true;
+    }
+
+    bool PostTraverseBinLAnd(BinaryOperator*, bool)
+    {
+        assert(!ignoredInAssert_.empty());
+        ignoredInAssert_.pop();
+        return true;
+    }
+
+    bool TraverseBinLAnd(BinaryOperator* expr)
+    {
+        bool ret = true;
+        if (PreTraverseBinLAnd(expr))
+        {
+            ret = FilteringPlugin::TraverseBinLAnd(expr);
+            PostTraverseBinLAnd(expr, ret);
+        }
+        return ret;
+    }
+
+    bool VisitImplicitCastExpr(ImplicitCastExpr const* expr)
+    {
+        if (ignoreLocation(expr))
+        {
+            return true;
+        }
+        if (!expr->getType()->isBooleanType())
+        {
+            return true;
+        }
+        auto const sub = expr->getSubExpr();
+        auto const t = sub->getType();
+        if (t->isBooleanType())
+        {
+            return true;
+        }
+        if (sub->isValueDependent())
+        {
+            return true;
+        }
+        APValue res;
+        if (!sub->isCXX11ConstantExpr(compiler.getASTContext(), &res))
+        {
+            return true;
+        }
+        auto const l = expr->getExprLoc();
+        if (!ignoredInAssert_.empty() && expr == ignoredInAssert_.top())
+        {
+            if (auto const e = dyn_cast<clang::StringLiteral>(sub->IgnoreParenImpCasts()))
+            {
+                if (e->isAscii()) // somewhat randomly restrict to plain literals
+                {
+                    if (compiler.getSourceManager().isMacroArgExpansion(l)
+                        && Lexer::getImmediateMacroName(l, compiler.getSourceManager(),
+                                                        compiler.getLangOpts())
+                               == "assert")
+                    {
+                        //TODO: only ignore outermost '!"..."' or '... && "..."'
+                        return true;
+                    }
+                }
+            }
+        }
+        auto l1 = l;
+        if (compiler.getSourceManager().isMacroBodyExpansion(l1))
+        {
+            auto const n = Lexer::getImmediateMacroName(l1, compiler.getSourceManager(),
+                                                        compiler.getLangOpts());
+            if (n == "FALSE" || n == "TRUE" || n == "sal_False" || n == "sal_True")
+            {
+                l1 = compiler.getSourceManager().getImmediateMacroCallerLoc(l1);
+            }
+            // For exmaple, /usr/include/glib-2.0/glib/gmacros.h from
+            // glib2-devel-2.62.1-1.fc31.x86_64 has
+            //
+            //   #define TRUE (!FALSE)
+            //
+            // so handle that wrapped macro body expansion, too:
+            if (compiler.getSourceManager().isMacroBodyExpansion(l1)
+                && Lexer::getImmediateMacroName(l1, compiler.getSourceManager(),
+                                                compiler.getLangOpts())
+                       == "TRUE")
+            {
+                l1 = compiler.getSourceManager().getImmediateMacroCallerLoc(l1);
+            }
+        }
+        if (isSharedCAndCppCode(l1))
+        {
+            // Cover just enough cases to handle things like `while (0)` or the use of `sal_True` in
+            //
+            //   #define OSL_FAIL(m) SAL_DETAIL_WARN_IF_FORMAT(sal_True, "legacy.osl", "%s", m)
+            //
+            // in include/osl/diagnose.h:
+            if (auto const t1 = t->getAs<BuiltinType>())
+            {
+                if (t1->getKind() == BuiltinType::Int)
+                {
+                    auto const& v = res.getInt();
+                    if (v == 0 || v == 1)
+                    {
+                        return true;
+                    }
+                }
+            }
+            if (loplugin::TypeCheck(t).Typedef("sal_Bool").GlobalNamespace())
+            {
+                return true;
+            }
+        }
+        bool suggestion;
+        bool replacement;
+        if (res.isInt())
+        {
+            suggestion = true;
+            replacement = res.getInt() != 0;
+        }
+        else if (res.isFloat())
+        {
+            suggestion = true;
+            replacement = !res.getFloat().isZero();
+        }
+        else if (res.isNullPointer())
+        {
+            suggestion = true;
+            replacement = false;
+        }
+        else
+        {
+            suggestion = false;
+        }
+        report(DiagnosticsEngine::Warning,
+               "implicit conversion of constant %0 of type %1 to 'bool'%select{|; use "
+               "'%select{false|true}3' instead}2",
+               l)
+            << res.getAsString(compiler.getASTContext(), t) << t << suggestion << replacement
+            << expr->getSourceRange();
+        return true;
+    }
+
+    bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
+
+private:
+    std::stack<Expr const*> ignoredInAssert_;
+    unsigned int externCContexts_ = 0;
+
+    void run() override
+    {
+        if (preRun())
+        {
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+        }
+    }
+
+    bool isFromCIncludeFile(SourceLocation spellingLocation) const
+    {
+        return !compiler.getSourceManager().isInMainFile(spellingLocation)
+               && (StringRef(
+                       compiler.getSourceManager().getPresumedLoc(spellingLocation).getFilename())
+                       .endswith(".h"));
+    }
+
+    bool isSharedCAndCppCode(SourceLocation location) const
+    {
+        while (compiler.getSourceManager().isMacroArgExpansion(location))
+        {
+            location = compiler.getSourceManager().getImmediateMacroCallerLoc(location);
+        }
+        // Assume that code is intended to be shared between C and C++ if it comes from an include
+        // file ending in .h, and is either in an extern "C" context  or the body of a macro
+        // definition:
+        return isFromCIncludeFile(compiler.getSourceManager().getSpellingLoc(location))
+               && (externCContexts_ != 0
+                   || compiler.getSourceManager().isMacroBodyExpansion(location));
+    }
+};
+
+loplugin::Plugin::Registration<ConstToBool> consttobool("consttobool");
+}
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/consttobool.cxx b/compilerplugins/clang/test/consttobool.cxx
new file mode 100644
index 000000000000..6110b4ba0942
--- /dev/null
+++ b/compilerplugins/clang/test/consttobool.cxx
@@ -0,0 +1,49 @@
+/* -*- 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/config.h>
+
+#include <sal/types.h>
+
+#pragma clang diagnostic ignored "-Wnull-conversion"
+
+enum E
+{
+    E0,
+    E1,
+    E2
+};
+
+int const c1 = 1;
+constexpr int c2 = 2;
+
+int main()
+{
+    bool b;
+    // expected-error@+1 {{implicit conversion of constant 0 of type 'int' to 'bool'; use 'false' instead [loplugin:consttobool]}}
+    b = 0;
+    // expected-error@+1 {{implicit conversion of constant 1 of type 'sal_Bool' (aka 'unsigned char') to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = sal_True;
+    // expected-error-re@+1 {{implicit conversion of constant {{nullptr|0}} of type 'nullptr_t' to 'bool'; use 'false' instead [loplugin:consttobool]}}
+    b = nullptr;
+    // expected-error@+1 {{implicit conversion of constant 1.000000e+00 of type 'double' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = 1.0;
+    // expected-error@+1 {{implicit conversion of constant 2 of type 'E' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = E2;
+    // expected-error@+1 {{implicit conversion of constant 97 of type 'char' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = 'a';
+    // expected-error@+1 {{implicit conversion of constant 1 of type 'int' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = c1;
+    // expected-error@+1 {{implicit conversion of constant 2 of type 'int' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = c2;
+    // expected-error@+1 {{implicit conversion of constant 3 of type 'int' to 'bool'; use 'true' instead [loplugin:consttobool]}}
+    b = (c1 | c2);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */