Skip to content

Commit

Permalink
[clang-tidy] Be more liberal about literal zeroes in abseil checks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Hyrum Wright committed Dec 21, 2018
1 parent e5b64ea commit 9fc3a5f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 6 deletions.
Expand Up @@ -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(
Expand Down
42 changes: 36 additions & 6 deletions clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
Expand Up @@ -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>
Expand Down
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 9fc3a5f

Please sign in to comment.