Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[clang-tidy] Move incorrect-roundings to upstream.
Summary: This is originally implemented by Jacques Pienaar. Reviewers: alexfh Subscribers: cfe-commits, jpienaar Differential Revision: http://reviews.llvm.org/D16764 llvm-svn: 260084
- Loading branch information
Showing
7 changed files
with
216 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 12 additions & 0 deletions
12
clang-tools-extra/docs/clang-tidy/checks/misc-incorrect-roundings.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
86 changes: 86 additions & 0 deletions
86
clang-tools-extra/test/clang-tidy/misc-incorrect-roundings.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)); | ||
} |