summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHyrum Wright <hwright@google.com>2018-12-21 21:07:11 +0000
committerHyrum Wright <hwright@google.com>2018-12-21 21:07:11 +0000
commit9fc3a5f4383408f9baf660c28220285dbea29cbc (patch)
tree131ffeddc4fa32fde19c7bfbcf2d958928486e9d
parente5b64ea2b81334295f2a31f7f9fc42aa8afc7748 (diff)
downloadbcm5719-llvm-9fc3a5f4383408f9baf660c28220285dbea29cbc.tar.gz
bcm5719-llvm-9fc3a5f4383408f9baf660c28220285dbea29cbc.zip
[clang-tidy] Be more liberal about literal zeroes in abseil checks
Summary: Previously, we'd only match on literal floating or integral zeroes, but I've now also learned that some users spell that value as int{0} or float{0}, which also need to be matched. Differential Revision: https://reviews.llvm.org/D56012 llvm-svn: 349953
-rw-r--r--clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp4
-rw-r--r--clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp42
-rw-r--r--clang-tools-extra/test/clang-tidy/abseil-duration-factory-scale.cpp14
3 files changed, 54 insertions, 6 deletions
diff --git a/clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp b/clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
index 078f88cff44..e56ed8198e9 100644
--- a/clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
@@ -123,6 +123,10 @@ void DurationFactoryScaleCheck::registerMatchers(MatchFinder *Finder) {
hasArgument(
0,
ignoringImpCasts(anyOf(
+ cxxFunctionalCastExpr(
+ hasDestinationType(
+ anyOf(isInteger(), realFloatingPointType())),
+ hasSourceExpression(initListExpr())),
integerLiteral(equals(0)), floatLiteral(equals(0.0)),
binaryOperator(hasOperatorName("*"),
hasEitherOperand(ignoringImpCasts(
diff --git a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
index 3f3affc5d3e..a7254779936 100644
--- a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
@@ -105,14 +105,44 @@ llvm::StringRef getFactoryForScale(DurationScale Scale) {
llvm_unreachable("unknown scaling factor");
}
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr())))
+/// \code
+/// int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+ ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+ return N < Node.getNumInits() &&
+ InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+ Builder);
+}
+
/// Returns `true` if `Node` is a value which evaluates to a literal `0`.
bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
- return selectFirst<const clang::Expr>(
- "val",
- match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)),
- floatLiteral(equals(0.0)))))
- .bind("val"),
- Node, *Result.Context)) != nullptr;
+ auto ZeroMatcher =
+ anyOf(integerLiteral(equals(0)), floatLiteral(equals(0.0)));
+
+ // Check to see if we're using a zero directly.
+ if (selectFirst<const clang::Expr>(
+ "val", match(expr(ignoringImpCasts(ZeroMatcher)).bind("val"), Node,
+ *Result.Context)) != nullptr)
+ return true;
+
+ // Now check to see if we're using a functional cast with a scalar
+ // initializer expression, e.g. `int{0}`.
+ if (selectFirst<const clang::Expr>(
+ "val",
+ match(cxxFunctionalCastExpr(
+ hasDestinationType(
+ anyOf(isInteger(), realFloatingPointType())),
+ hasSourceExpression(initListExpr(hasInit(0, ZeroMatcher))))
+ .bind("val"),
+ Node, *Result.Context)) != nullptr)
+ return true;
+
+ return false;
}
llvm::Optional<std::string>
diff --git a/clang-tools-extra/test/clang-tidy/abseil-duration-factory-scale.cpp b/clang-tools-extra/test/clang-tidy/abseil-duration-factory-scale.cpp
index dc94773f5ac..04c361328f5 100644
--- a/clang-tools-extra/test/clang-tidy/abseil-duration-factory-scale.cpp
+++ b/clang-tools-extra/test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -2,6 +2,9 @@
#include "absl/time/time.h"
+namespace std { typedef long long int64_t; }
+using int64_t = std::int64_t;
+
void ScaleTest() {
absl::Duration d;
@@ -30,6 +33,15 @@ void ScaleTest() {
d = absl::Seconds(0x0.000001p-126f);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
// CHECK-FIXES: absl::ZeroDuration();
+ d = absl::Seconds(int{0});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+ // CHECK-FIXES: absl::ZeroDuration();
+ d = absl::Seconds(int64_t{0});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+ // CHECK-FIXES: absl::ZeroDuration();
+ d = absl::Seconds(float{0});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+ // CHECK-FIXES: absl::ZeroDuration();
// Fold seconds into minutes
d = absl::Seconds(30 * 60);
@@ -83,6 +95,8 @@ void ScaleTest() {
// None of these should trigger the check
d = absl::Seconds(60);
+ d = absl::Seconds(int{60});
+ d = absl::Seconds(float{60});
d = absl::Seconds(60 + 30);
d = absl::Seconds(60 - 30);
d = absl::Seconds(50 * 30);
OpenPOWER on IntegriCloud