[PATCH] D56012: Be more liberal about literal zeroes in abseil checks

[Hyrum Wright via Phabricator via cfe-commits]

hwright created this revision.
hwright added reviewers: aaron.ballman, JonasToth, alexfh, hokein.
hwright added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56012

Files:
  clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  test/clang-tidy/abseil-duration-factory-scale.cpp


Index: test/clang-tidy/abseil-duration-factory-scale.cpp
===================================================================
--- test/clang-tidy/abseil-duration-factory-scale.cpp
+++ test/clang-tidy/abseil-duration-factory-scale.cpp
@@ -1,5 +1,6 @@
 // RUN: %check_clang_tidy %s abseil-duration-factory-scale %t -- -- -I%S/Inputs
 
+#include <stdint.h>
 #include "absl/time/time.h"
 
 void ScaleTest() {
@@ -30,6 +31,15 @@
   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 +93,8 @@
 
   // 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);
Index: clang-tidy/abseil/DurationRewriter.cpp
===================================================================
--- clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tidy/abseil/DurationRewriter.cpp
@@ -105,14 +105,37 @@
   llvm_unreachable("unknown scaling factor");
 }
 
+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>
Index: clang-tidy/abseil/DurationFactoryScaleCheck.cpp
===================================================================
--- clang-tidy/abseil/DurationFactoryScaleCheck.cpp
+++ clang-tidy/abseil/DurationFactoryScaleCheck.cpp
@@ -123,6 +123,10 @@
           hasArgument(
               0,
               ignoringImpCasts(anyOf(
+                  cxxFunctionalCastExpr(
+                      hasDestinationType(
+                          anyOf(isInteger(), realFloatingPointType())),
+                      hasSourceExpression(initListExpr())),
                   integerLiteral(equals(0)), floatLiteral(equals(0.0)),
                   binaryOperator(hasOperatorName("*"),
                                  hasEitherOperand(ignoringImpCasts(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56012.179306.patch
Type: text/x-patch
Size: 4224 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181221/c1775043/attachment.bin>