diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2014-01-17 17:12:36 +0100 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2014-01-17 18:45:16 +0100 |
commit | e908f69ec0de53b6e8bad9dc80ceb2004a68c09b (patch) | |
tree | 40036b1b120352600926dbc783aeb8b2875def33 /compilerplugins | |
parent | bool improvements (diff) | |
download | core-e908f69ec0de53b6e8bad9dc80ceb2004a68c09b.tar.gz core-e908f69ec0de53b6e8bad9dc80ceb2004a68c09b.zip |
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.cxx | 480 |
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: */ |