summaryrefslogtreecommitdiff
path: root/compilerplugins
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2014-01-17 17:12:36 +0100
committerStephan Bergmann <sbergman@redhat.com>2014-01-17 18:45:16 +0100
commite908f69ec0de53b6e8bad9dc80ceb2004a68c09b (patch)
tree40036b1b120352600926dbc783aeb8b2875def33 /compilerplugins
parentc963d7e642c24f40c19fb9dc227db5da96728c12 (diff)
Clang plugin that flags implicit conversions from bool
...as they are often enough errors, like a typo in brace placement in if (foo == (FOO || bar == BAR) && baz) or a literal true passed as an argument to a function that rather expects an integer bit mask, etc. The plugin is smart enough to detect interaction with logically boolean return/parameter types of C functions that use [unsigned] int instead, and knows the relevant boolean typedefs (sal_Bool, gboolean, etc.). Change-Id: I5f0e4344fe86589bec35a71018c7effdedf85e3e
Diffstat (limited to 'compilerplugins')
-rw-r--r--compilerplugins/clang/implicitboolconversion.cxx480
1 files changed, 480 insertions, 0 deletions
diff --git a/compilerplugins/clang/implicitboolconversion.cxx b/compilerplugins/clang/implicitboolconversion.cxx
new file mode 100644
index 000000000000..98cb94c97102
--- /dev/null
+++ b/compilerplugins/clang/implicitboolconversion.cxx
@@ -0,0 +1,480 @@
+/* -*- 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 <algorithm>
+#include <cassert>
+#include <cstddef>
+#include <iterator>
+#include <stack>
+#include <string>
+#include <vector>
+
+#include "plugin.hxx"
+
+template<> struct std::iterator_traits<ExprIterator> {
+ typedef std::ptrdiff_t difference_type;
+ typedef Expr * value_type;
+ typedef Expr const ** pointer;
+ typedef Expr const & reference;
+ typedef std::random_access_iterator_tag iterator_category;
+};
+
+namespace {
+
+bool isBool(Expr const * expr) {
+ QualType t1 { expr->getType() };
+ if (t1->isBooleanType()) {
+ return true;
+ }
+// css::uno::Sequence<sal_Bool> s(1);s[0]=false /*unotools/source/config/configitem.cxx*/:
+if(t1->isSpecificBuiltinType(BuiltinType::UChar))return true;
+ TypedefType const * t2 = t1->getAs<TypedefType>();
+ if (t2 == nullptr) {
+ return false;
+ }
+ std::string name(t2->getDecl()->getNameAsString());
+ return name == "sal_Bool" || name == "FcBool" || name == "UBool"
+ || name == "dbus_bool_t" || name == "gboolean" || name == "hb_bool_t";
+}
+
+bool isBoolExpr(Expr const * expr) {
+ if (isBool(expr)) {
+ return true;
+ }
+ ConditionalOperator const * co = dyn_cast<ConditionalOperator>(expr);
+ if (co != nullptr) {
+ ImplicitCastExpr const * ic1 = dyn_cast<ImplicitCastExpr>(
+ co->getTrueExpr()->IgnoreParens());
+ ImplicitCastExpr const * ic2 = dyn_cast<ImplicitCastExpr>(
+ co->getFalseExpr()->IgnoreParens());
+ if (ic1 != nullptr && ic2 != nullptr
+ && ic1->getType()->isSpecificBuiltinType(BuiltinType::Int)
+ && isBoolExpr(ic1->getSubExpr()->IgnoreParens())
+ && ic2->getType()->isSpecificBuiltinType(BuiltinType::Int)
+ && isBoolExpr(ic2->getSubExpr()->IgnoreParens()))
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
+class ImplicitBoolConversion:
+ public RecursiveASTVisitor<ImplicitBoolConversion>, public loplugin::Plugin
+{
+public:
+ explicit ImplicitBoolConversion(CompilerInstance & compiler):
+ Plugin(compiler) {}
+
+ virtual void run() override
+ { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+ bool TraverseCallExpr(CallExpr * expr);
+
+ bool TraverseCStyleCastExpr(CStyleCastExpr * expr);
+
+ bool TraverseCXXStaticCastExpr(CXXStaticCastExpr * expr);
+
+ bool TraverseCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr);
+
+ bool TraverseConditionalOperator(ConditionalOperator * expr);
+
+ bool TraverseBinLT(BinaryOperator * expr);
+
+ bool TraverseBinLE(BinaryOperator * expr);
+
+ bool TraverseBinGT(BinaryOperator * expr);
+
+ bool TraverseBinGE(BinaryOperator * expr);
+
+ bool TraverseBinEQ(BinaryOperator * expr);
+
+ bool TraverseBinNE(BinaryOperator * expr);
+
+ bool TraverseBinAssign(BinaryOperator * expr);
+
+ bool TraverseBinAndAssign(CompoundAssignOperator * expr);
+
+ bool TraverseBinOrAssign(CompoundAssignOperator * expr);
+
+ bool TraverseBinXorAssign(CompoundAssignOperator * expr);
+
+ bool TraverseReturnStmt(ReturnStmt * stmt);
+
+ bool TraverseFunctionDecl(FunctionDecl * decl);
+
+ bool VisitImplicitCastExpr(ImplicitCastExpr const * expr);
+
+private:
+ void reportWarning(ImplicitCastExpr const * expr);
+
+ std::stack<std::vector<ImplicitCastExpr const *>> nested;
+ bool externCIntFunctionDefinition = false;
+};
+
+bool ImplicitBoolConversion::TraverseCallExpr(CallExpr * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseCallExpr(expr);
+ Decl const * d = expr->getCalleeDecl();
+ bool ext = false;
+ FunctionProtoType const * t = nullptr;
+ if (d != nullptr) {
+ FunctionDecl const * fd = dyn_cast<FunctionDecl>(d);
+ if (fd != nullptr && (fd->isExternC() || fd->isInExternCContext())) {
+ ext = true;
+ PointerType const * pt = dyn_cast<PointerType>(fd->getType());
+ t = (pt == nullptr ? fd->getType() : pt->getPointeeType())
+ ->getAs<FunctionProtoType>();
+ } else {
+ VarDecl const * vd = dyn_cast<VarDecl>(d);
+ if (vd != nullptr && (vd->isExternC() || vd->isInExternCContext()))
+ {
+ ext = true;
+ PointerType const * pt = dyn_cast<PointerType>(vd->getType());
+ t = (pt == nullptr ? vd->getType() : pt->getPointeeType())
+ ->getAs<FunctionProtoType>();
+ }
+ }
+ }
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (ext) {
+ auto j = std::find_if(
+ expr->arg_begin(), expr->arg_end(),
+ [&i](Expr * e) { return i == e->IgnoreParens(); });
+ if (j == expr->arg_end()) {
+ reportWarning(i);
+ } else {
+ std::ptrdiff_t n = j - expr->arg_begin();
+ assert(n >= 0);
+ assert(n < t->getNumArgs() || t->isVariadic());
+ if (n < t->getNumArgs()
+ && !(t->getArgType(n)->isSpecificBuiltinType(
+ BuiltinType::Int)
+ || (t->getArgType(n)->isSpecificBuiltinType(
+ BuiltinType::UInt))))
+ {
+ reportWarning(i);
+ }
+ }
+ } else {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseCStyleCastExpr(CStyleCastExpr * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseCStyleCastExpr(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getSubExpr()->IgnoreParens()) {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseCXXStaticCastExpr(CXXStaticCastExpr * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseCXXStaticCastExpr(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getSubExpr()->IgnoreParens()) {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseCXXFunctionalCastExpr(
+ CXXFunctionalCastExpr * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseCXXFunctionalCastExpr(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getSubExpr()->IgnoreParens()) {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseConditionalOperator(
+ ConditionalOperator * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseConditionalOperator(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getTrueExpr()->IgnoreParens()
+ && isBoolExpr(expr->getFalseExpr()->IgnoreParenImpCasts()))
+ || (i == expr->getFalseExpr()->IgnoreParens()
+ && isBoolExpr(expr->getTrueExpr()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinLT(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinLT(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinLE(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinLE(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinGT(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinGT(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinGE(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinGE(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinEQ(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinEQ(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinNE(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinNE(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (!((i == expr->getLHS()->IgnoreParens()
+ && isBool(expr->getRHS()->IgnoreParenImpCasts()))
+ || (i == expr->getRHS()->IgnoreParens()
+ && isBool(expr->getLHS()->IgnoreParenImpCasts()))))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+// /usr/include/gtk-2.0/gtk/gtktogglebutton.h: struct _GtkToggleButton:
+// guint GSEAL (active) : 1;
+// even though <http://www.gtk.org/api/2.6/gtk/GtkToggleButton.html>:
+// "active" gboolean : Read / Write
+bool ImplicitBoolConversion::TraverseBinAssign(BinaryOperator * expr) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinAssign(expr);
+ bool ext = false;
+ MemberExpr const * me = dyn_cast<MemberExpr>(expr->getLHS());
+ if (me != nullptr) {
+ FieldDecl const * fd = dyn_cast<FieldDecl>(me->getMemberDecl());
+ if (fd != nullptr && fd->isBitField()
+ && fd->getBitWidthValue(compiler.getASTContext()) == 1)
+ {
+ TypedefType const * t = fd->getType()->getAs<TypedefType>();
+ ext = t != nullptr && t->getDecl()->getNameAsString() == "guint";
+ }
+ }
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getRHS()->IgnoreParens() || !ext) {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinAndAssign(CompoundAssignOperator * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinAndAssign(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getRHS()->IgnoreParens()
+ || !isBool(expr->getLHS()->IgnoreParens()))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinOrAssign(CompoundAssignOperator * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinOrAssign(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getRHS()->IgnoreParens()
+ || !isBool(expr->getLHS()->IgnoreParens()))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseBinXorAssign(CompoundAssignOperator * expr)
+{
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseBinXorAssign(expr);
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr->getRHS()->IgnoreParens()
+ || !isBool(expr->getLHS()->IgnoreParens()))
+ {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseReturnStmt(ReturnStmt * stmt) {
+ nested.push(std::vector<ImplicitCastExpr const *>());
+ bool ret = RecursiveASTVisitor::TraverseReturnStmt(stmt);
+ Expr const * expr = stmt->getRetValue();
+ if (expr != nullptr) {
+ ExprWithCleanups const * ec = dyn_cast<ExprWithCleanups>(expr);
+ if (ec != nullptr) {
+ expr = ec->getSubExpr();
+ }
+ expr = expr->IgnoreParens();
+ }
+ assert(!nested.empty());
+ for (auto i: nested.top()) {
+ if (i != expr || !externCIntFunctionDefinition) {
+ reportWarning(i);
+ }
+ }
+ nested.pop();
+ return ret;
+}
+
+bool ImplicitBoolConversion::TraverseFunctionDecl(FunctionDecl * decl) {
+ bool ext = (decl->isExternC() || decl->isInExternCContext())
+ && decl->isThisDeclarationADefinition()
+ && (decl->getResultType()->isSpecificBuiltinType(BuiltinType::Int)
+ || decl->getResultType()->isSpecificBuiltinType(BuiltinType::UInt));
+ if (ext) {
+ assert(!externCIntFunctionDefinition);
+ externCIntFunctionDefinition = true;
+ }
+ bool ret = RecursiveASTVisitor::TraverseFunctionDecl(decl);
+ if (ext) {
+ externCIntFunctionDefinition = false;
+ }
+ return ret;
+}
+
+bool ImplicitBoolConversion::VisitImplicitCastExpr(
+ ImplicitCastExpr const * expr)
+{
+ if (ignoreLocation(expr)) {
+ return true;
+ }
+ if (expr->getSubExprAsWritten()->getType()->isBooleanType()
+ && !isBool(expr))
+ {
+ if (nested.empty()) {
+ reportWarning(expr);
+ } else {
+ nested.top().push_back(expr);
+ }
+ }
+ return true;
+}
+
+void ImplicitBoolConversion::reportWarning(ImplicitCastExpr const * expr) {
+ report(
+ DiagnosticsEngine::Warning,
+ "implicit conversion (%0) from bool to %1", expr->getLocStart())
+ << expr->getCastKindName() << expr->getType() << expr->getSourceRange();
+}
+
+loplugin::Plugin::Registration<ImplicitBoolConversion> X(
+ "implicitboolconversion");
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */