diff options
author | Hyrum Wright <hwright@google.com> | 2018-12-21 21:07:11 +0000 |
---|---|---|
committer | Hyrum Wright <hwright@google.com> | 2018-12-21 21:07:11 +0000 |
commit | 9fc3a5f4383408f9baf660c28220285dbea29cbc (patch) | |
tree | 131ffeddc4fa32fde19c7bfbcf2d958928486e9d | |
parent | e5b64ea2b81334295f2a31f7f9fc42aa8afc7748 (diff) | |
download | bcm5719-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
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); |