diff options
author | Noel Grandin <noel@peralex.com> | 2016-02-01 14:55:56 +0200 |
---|---|---|
committer | Noel Grandin <noelgrandin@gmail.com> | 2016-02-04 06:41:35 +0000 |
commit | 4e9b528dccdfc438394b1ffe029f1fc8178a6086 (patch) | |
tree | 7c8afdd29d6ee6b63d6e7c2b0a9026739b5351f0 /compilerplugins | |
parent | 4fa4b9addfaa4291096cb6de5d98c6612c36757c (diff) |
new loplugin fpcomparison
Find code that compares floating point values with == or !=
It should rather use rtl::math::approxEqual
Change-Id: I9026e08823340fa1d6a042c430515344c93215bd
Reviewed-on: https://gerrit.libreoffice.org/21997
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
Diffstat (limited to 'compilerplugins')
-rw-r--r-- | compilerplugins/clang/fpcomparison.cxx | 146 |
1 files changed, 146 insertions, 0 deletions
diff --git a/compilerplugins/clang/fpcomparison.cxx b/compilerplugins/clang/fpcomparison.cxx new file mode 100644 index 000000000000..b374da7a5da7 --- /dev/null +++ b/compilerplugins/clang/fpcomparison.cxx @@ -0,0 +1,146 @@ +/* -*- 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 "plugin.hxx" +#include "compat.hxx" + +/** +comparing floating point numbers using == or != is a bad idea. +*/ + +namespace { + +class FpComparison: + public RecursiveASTVisitor<FpComparison>, public loplugin::Plugin +{ +public: + explicit FpComparison(InstantiationData const & data): Plugin(data) {} + + virtual void run() override + { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitBinaryOperator(const BinaryOperator* ); + bool TraverseFunctionDecl(FunctionDecl* ); +private: + bool ignore(FunctionDecl* ); + enum class EState { None, TraverseProcess, TraverseIgnore }; + EState meState = EState::None; +}; + +static const std::set<std::string> whitelist { + "rtl::math::approxEqual", + "(anonymous namespace)::doubleToString", + "(anonymous namespace)::stringToDouble", + "rtl_math_round", + "rtl_math_approxValue", + "rtl_math_asinh", + "rtl_math_acosh", + "cppu::_equalSequence", // cppu/source/uno/eq.hxx + "cppu::_equalData", // cppu/source/uno/eq.hxx + "xmlscript::equalFont", // xmlscript/source/xmldlg_imexp/xmldlg_export.cxx + + // these might need fixing + "basegfx::tools::getSmallestDistancePointToPolygon", // basegfx/source/polygon/b2dpolygontools.cxx + "basegfx::tools::getSmallestDistancePointToPolyPolygon", // basegfx/source/polygon/b2dpolypolygontools.cxx + "bridge_test::performTest", // testtools/source/bridgetest/bridgetest.cxx + "bridge_test::equals", + "(anonymous namespace)::lcl_getNANInsteadDBL_MIN", // chart2/source/controller/chartapiwrapper/ChartDataWrapper.cxx +}; +bool FpComparison::TraverseFunctionDecl(FunctionDecl* function) +{ + bool bIgnore = ignore(function); + meState = bIgnore ? EState::TraverseIgnore : EState::TraverseProcess; + bool bRet = RecursiveASTVisitor::TraverseFunctionDecl(function); + meState = EState::None; + return bRet; +} + +bool FpComparison::ignore(FunctionDecl* function) +{ + if (ignoreLocation(function)) { + return true; + } + // we assume that these modules know what they are doing with FP stuff + StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(function->getLocStart())); + if (aFileName.startswith(SRCDIR "/sc/")) { + return true; + } + if (!function->doesThisDeclarationHaveABody()) { + return true; + } + // Ignore operator== and operator!= + if (function->getOverloadedOperator() == OO_EqualEqual + || function->getOverloadedOperator() == OO_ExclaimEqual) { + return true; + } + // ignore known good functions + std::string s = function->getQualifiedNameAsString(); + if (whitelist.find(s) != whitelist.end()) { + return true; + } +// cout << "xxx " + function->getQualifiedNameAsString() << endl; + return false; +} + +static bool isZeroConstant(ASTContext& context, const Expr* expr) +{ + // calling isCXX11ConstantExpr with non-arithmetic types sometimes results in a crash + if (!expr->getType()->isArithmeticType()) { + return false; + } + // prevent clang crash + if (!context.getLangOpts().CPlusPlus) { + return false; + } + APValue result; + if (expr->isCXX11ConstantExpr(context, &result) + && result.isFloat() && result.getFloat().isZero()) + { + return true; + } + return false; +} +bool FpComparison::VisitBinaryOperator(const BinaryOperator* binaryOp) +{ + if (meState != EState::TraverseProcess || ignoreLocation(binaryOp)) { + return true; + } + if (binaryOp->getOpcode() != BO_EQ && binaryOp->getOpcode() != BO_NE) { + return true; + } + // comparison with zero is valid + if (isZeroConstant(compiler.getASTContext(), binaryOp->getLHS()) + || isZeroConstant(compiler.getASTContext(), binaryOp->getRHS())) + { + return true; + } + QualType LHSStrippedType = binaryOp->getLHS()->IgnoreParenImpCasts()->getType(); + QualType RHSStrippedType = binaryOp->getRHS()->IgnoreParenImpCasts()->getType(); + if (LHSStrippedType->isFloatingType() && RHSStrippedType->isFloatingType()) { + report( + DiagnosticsEngine::Warning, "floating-point comparison", + binaryOp->getSourceRange().getBegin()) + << binaryOp->getSourceRange(); + } + return true; +} + + +loplugin::Plugin::Registration< FpComparison > X("fpcomparison", true); + +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ |