diff options
7 files changed, 216 insertions, 1 deletions
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 6835c549c73..93f8af9f39d 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -7,11 +7,12 @@ add_clang_library(clangTidyMiscModule BoolPointerImplicitConversionCheck.cpp DefinitionsInHeadersCheck.cpp InaccurateEraseCheck.cpp + IncorrectRoundings.cpp InefficientAlgorithmCheck.cpp MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp - MoveConstantArgumentCheck.cpp + MoveConstantArgumentCheck.cpp MoveConstructorInitCheck.cpp NewDeleteOverloadsCheck.cpp NoexceptMoveConstructorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.cpp b/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.cpp new file mode 100644 index 00000000000..993af51d53e --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.cpp @@ -0,0 +1,74 @@ +//===--- IncorrectRoundings.cpp - clang-tidy ------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "IncorrectRoundings.h" +#include "clang/AST/DeclBase.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +namespace clang { +namespace ast_matchers { +AST_MATCHER(FloatingLiteral, floatHalf) { + const auto &literal = Node.getValue(); + if ((&Node.getSemantics()) == &llvm::APFloat::IEEEsingle) + return literal.convertToFloat() == 0.5f; + if ((&Node.getSemantics()) == &llvm::APFloat::IEEEdouble) + return literal.convertToDouble() == 0.5; + return false; +} + +// TODO(hokein): Moving it to ASTMatchers.h +AST_MATCHER(BuiltinType, isFloatingPoint) { + return Node.isFloatingPoint(); +} +} // namespace ast_matchers +} // namespace clang + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +void IncorrectRoundings::registerMatchers(MatchFinder *MatchFinder) { + // Match a floating literal with value 0.5. + auto FloatHalf = floatLiteral(floatHalf()); + + // Match a floating point expression. + auto FloatType = expr(hasType(builtinType(isFloatingPoint()))); + + // Match a floating literal of 0.5 or a floating literal of 0.5 implicitly. + // cast to floating type. + auto FloatOrCastHalf = + anyOf(FloatHalf, implicitCastExpr(FloatType, has(FloatHalf))); + + // Match if either the LHS or RHS is a floating literal of 0.5 or a floating + // literal of 0.5 and the other is of type double or vice versa. + auto OneSideHalf = anyOf(allOf(hasLHS(FloatOrCastHalf), hasRHS(FloatType)), + allOf(hasRHS(FloatOrCastHalf), hasLHS(FloatType))); + + // Find expressions of cast to int of the sum of a floating point expression + // and 0.5. + MatchFinder->addMatcher( + implicitCastExpr( + hasImplicitDestinationType(isInteger()), + ignoringParenCasts(binaryOperator(hasOperatorName("+"), OneSideHalf))) + .bind("CastExpr"), + this); +} + +void IncorrectRoundings::check(const MatchFinder::MatchResult &Result) { + const auto *CastExpr = Result.Nodes.getStmtAs<ImplicitCastExpr>("CastExpr"); + diag(CastExpr->getLocStart(), + "casting (double + 0.5) to integer leads to incorrect rounding; " + "consider using lround (#include <cmath>) instead"); +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.h b/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.h new file mode 100644 index 00000000000..bb7cf144835 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/IncorrectRoundings.h @@ -0,0 +1,38 @@ +//===--- IncorrectRoundings.h - clang-tidy ----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INCORRECTROUNDINGS_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INCORRECTROUNDINGS_H_ + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Checks the usage of patterns known to produce incorrect rounding. +/// Programmers often use +/// (int)(double_expression + 0.5) +/// to round the double expression to an integer. The problem with this +/// 1. It is unnecessarily slow. +/// 2. It is incorrect. The number 0.499999975 (smallest representable float +/// number below 0.5) rounds to 1.0. Even worse behavior for negative +/// numbers where both -0.5f and -1.4f both round to 0.0. +class IncorrectRoundings : public ClangTidyCheck { +public: + IncorrectRoundings(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INCORRECTROUNDINGS_H_ diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index 62f6b0529fc..d34e35748ac 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -16,6 +16,7 @@ #include "BoolPointerImplicitConversionCheck.h" #include "DefinitionsInHeadersCheck.h" #include "InaccurateEraseCheck.h" +#include "IncorrectRoundings.h" #include "InefficientAlgorithmCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" @@ -54,6 +55,8 @@ public: "misc-definitions-in-headers"); CheckFactories.registerCheck<InaccurateEraseCheck>( "misc-inaccurate-erase"); + CheckFactories.registerCheck<IncorrectRoundings>( + "misc-incorrect-roundings"); CheckFactories.registerCheck<InefficientAlgorithmCheck>( "misc-inefficient-algorithm"); CheckFactories.registerCheck<MacroParenthesesCheck>( diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index aca429b0e43..42f7cf39022 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -49,6 +49,7 @@ Clang-Tidy Checks misc-bool-pointer-implicit-conversion misc-definitions-in-headers misc-inaccurate-erase + misc-incorrect-roundings misc-inefficient-algorithm misc-macro-parentheses misc-macro-repeated-side-effects diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-incorrect-roundings.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-incorrect-roundings.rst new file mode 100644 index 00000000000..abe70b412b2 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-incorrect-roundings.rst @@ -0,0 +1,12 @@ +misc-incorrect-roundings +======================== + +Checks the usage of patterns known to produce incorrect rounding. +Programmers often use + (int)(double_expression + 0.5) +to round the double expression to an integer. The problem with this: + +1. It is unnecessarily slow. +2. It is incorrect. The number 0.499999975 (smallest representable float + number below 0.5) rounds to 1.0. Even worse behavior for negative + numbers where both -0.5f and -1.4f both round to 0.0. diff --git a/clang-tools-extra/test/clang-tidy/misc-incorrect-roundings.cpp b/clang-tools-extra/test/clang-tidy/misc-incorrect-roundings.cpp new file mode 100644 index 00000000000..ae8638db02c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/misc-incorrect-roundings.cpp @@ -0,0 +1,86 @@ +// RUN: %check_clang_tidy %s misc-incorrect-roundings %t + +void b(int x) {} + +void f1() { + float f; + double d; + long double ld; + int x; + + x = (d + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) to integer leads to incorrect rounding; consider using lround (#include <cmath>) instead [misc-incorrect-roundings] + x = (d + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (f + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (f + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5 + d); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5f + d); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5 + ld); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5f + ld); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5 + f); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (0.5f + f); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: casting (double + 0.5) + x = (int)(d + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(d + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(ld + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(ld + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(f + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(f + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5 + d); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5f + d); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5 + ld); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5f + ld); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5 + f); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = (int)(0.5f + f); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: casting (double + 0.5) + x = static_cast<int>(d + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(d + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(ld + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(ld + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(f + 0.5); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(f + 0.5f); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5 + d); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5f + d); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5 + ld); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5f + ld); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5 + f); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + x = static_cast<int>(0.5f + f); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: casting (double + 0.5) + + // Don't warn if constant is not 0.5. + x = (int)(d + 0.6); + x = (int)(0.6 + d); + + // Don't warn if binary operator is not directly beneath cast. + x = (int)(1 + (0.5 + f)); +} |
