[clang-tools-extra] r347163 - Add the abseil-duration-factory-scale check.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 18 08:41:06 PST 2018


Author: aaronballman
Date: Sun Nov 18 08:41:06 2018
New Revision: 347163

URL: http://llvm.org/viewvc/llvm-project?rev=347163&view=rev
Log:
Add the abseil-duration-factory-scale check.

This check removes unneeded scaling of arguments when calling Abseil Time factory functions.

Patch by Hyrum Wright.

Added:
    clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
    clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-factory-scale.rst
    clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
    clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=347163&r1=347162&r2=347163&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Sun Nov 18 08:41:06 2018
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "DurationDivisionCheck.h"
 #include "DurationFactoryFloatCheck.h"
+#include "DurationFactoryScaleCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
@@ -30,6 +31,8 @@ public:
         "abseil-duration-division");
     CheckFactories.registerCheck<DurationFactoryFloatCheck>(
         "abseil-duration-factory-float");
+    CheckFactories.registerCheck<DurationFactoryScaleCheck>(
+        "abseil-duration-factory-scale");
     CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>(
         "abseil-faster-strsplit-delimiter");
     CheckFactories.registerCheck<NoInternalDependenciesCheck>(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=347163&r1=347162&r2=347163&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Sun Nov 18 08:41:06 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
   DurationDivisionCheck.cpp
   DurationFactoryFloatCheck.cpp
+  DurationFactoryScaleCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp

Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp?rev=347163&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp Sun Nov 18 08:41:06 2018
@@ -0,0 +1,269 @@
+//===--- DurationFactoryScaleCheck.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 "DurationFactoryScaleCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+namespace {
+
+// Potential scales of our inputs.
+enum class DurationScale {
+  Hours,
+  Minutes,
+  Seconds,
+  Milliseconds,
+  Microseconds,
+  Nanoseconds,
+};
+
+} // namespace
+
+// Given the name of a duration factory function, return the appropriate
+// `DurationScale` for that factory.  If no factory can be found for
+// `FactoryName`, return `None`.
+static llvm::Optional<DurationScale>
+getScaleForFactory(llvm::StringRef FactoryName) {
+  static const std::unordered_map<std::string, DurationScale> ScaleMap(
+      {{"Nanoseconds", DurationScale::Nanoseconds},
+       {"Microseconds", DurationScale::Microseconds},
+       {"Milliseconds", DurationScale::Milliseconds},
+       {"Seconds", DurationScale::Seconds},
+       {"Minutes", DurationScale::Minutes},
+       {"Hours", DurationScale::Hours}});
+
+  auto ScaleIter = ScaleMap.find(FactoryName);
+  if (ScaleIter == ScaleMap.end())
+    return llvm::None;
+
+  return ScaleIter->second;
+}
+
+// Given either an integer or float literal, return its value.
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+                       const FloatingLiteral *FloatLit) {
+  if (IntLit)
+    return IntLit->getValue().getLimitedValue();
+
+  assert(FloatLit != nullptr && "Neither IntLit nor FloatLit set");
+  return FloatLit->getValueAsApproximateDouble();
+}
+
+// Given the scale of a duration and a `Multiplier`, determine if `Multiplier`
+// would produce a new scale.  If so, return a tuple containing the new scale
+// and a suitable Multipler for that scale, otherwise `None`.
+static llvm::Optional<std::tuple<DurationScale, double>>
+GetNewScaleSingleStep(DurationScale OldScale, double Multiplier) {
+  switch (OldScale) {
+  case DurationScale::Hours:
+    if (Multiplier <= 1.0 / 60.0)
+      return std::make_tuple(DurationScale::Minutes, Multiplier * 60.0);
+    break;
+
+  case DurationScale::Minutes:
+    if (Multiplier >= 60.0)
+      return std::make_tuple(DurationScale::Hours, Multiplier / 60.0);
+    if (Multiplier <= 1.0 / 60.0)
+      return std::make_tuple(DurationScale::Seconds, Multiplier * 60.0);
+    break;
+
+  case DurationScale::Seconds:
+    if (Multiplier >= 60.0)
+      return std::make_tuple(DurationScale::Minutes, Multiplier / 60.0);
+    if (Multiplier <= 1e-3)
+      return std::make_tuple(DurationScale::Milliseconds, Multiplier * 1e3);
+    break;
+
+  case DurationScale::Milliseconds:
+    if (Multiplier >= 1e3)
+      return std::make_tuple(DurationScale::Seconds, Multiplier / 1e3);
+    if (Multiplier <= 1e-3)
+      return std::make_tuple(DurationScale::Microseconds, Multiplier * 1e3);
+    break;
+
+  case DurationScale::Microseconds:
+    if (Multiplier >= 1e3)
+      return std::make_tuple(DurationScale::Milliseconds, Multiplier / 1e3);
+    if (Multiplier <= 1e-3)
+      return std::make_tuple(DurationScale::Nanoseconds, Multiplier * 1e-3);
+    break;
+
+  case DurationScale::Nanoseconds:
+    if (Multiplier >= 1e3)
+      return std::make_tuple(DurationScale::Microseconds, Multiplier / 1e3);
+    break;
+  }
+
+  return llvm::None;
+}
+
+// Given the scale of a duration and a `Multiplier`, determine if `Multiplier`
+// would produce a new scale.  If so, return it, otherwise `None`.
+static llvm::Optional<DurationScale> GetNewScale(DurationScale OldScale,
+                                                 double Multiplier) {
+  while (Multiplier != 1.0) {
+    llvm::Optional<std::tuple<DurationScale, double>> result =
+        GetNewScaleSingleStep(OldScale, Multiplier);
+    if (!result)
+      break;
+    if (std::get<1>(*result) == 1.0)
+      return std::get<0>(*result);
+    Multiplier = std::get<1>(*result);
+    OldScale = std::get<0>(*result);
+  }
+
+  return llvm::None;
+}
+
+// Given a `Scale`, return the appropriate factory function call for
+// constructing a `Duration` for that scale.
+static llvm::StringRef GetFactoryForScale(DurationScale Scale) {
+  switch (Scale) {
+  case DurationScale::Hours:
+    return "absl::Hours";
+  case DurationScale::Minutes:
+    return "absl::Minutes";
+  case DurationScale::Seconds:
+    return "absl::Seconds";
+  case DurationScale::Milliseconds:
+    return "absl::Milliseconds";
+  case DurationScale::Microseconds:
+    return "absl::Microseconds";
+  case DurationScale::Nanoseconds:
+    return "absl::Nanoseconds";
+  }
+  llvm_unreachable("unknown scaling factor");
+}
+
+void DurationFactoryScaleCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(
+                     hasAnyName("::absl::Nanoseconds", "::absl::Microseconds",
+                                "::absl::Milliseconds", "::absl::Seconds",
+                                "::absl::Minutes", "::absl::Hours"))
+                     .bind("call_decl")),
+          hasArgument(
+              0,
+              ignoringImpCasts(anyOf(
+                  integerLiteral(equals(0)).bind("zero"),
+                  floatLiteral(equals(0.0)).bind("zero"),
+                  binaryOperator(hasOperatorName("*"),
+                                 hasEitherOperand(ignoringImpCasts(
+                                     anyOf(integerLiteral(), floatLiteral()))))
+                      .bind("mult_binop"),
+                  binaryOperator(hasOperatorName("/"), hasRHS(floatLiteral()))
+                      .bind("div_binop")))))
+          .bind("call"),
+      this);
+}
+
+void DurationFactoryScaleCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
+
+  // Don't try to replace things inside of macro definitions.
+  if (Call->getExprLoc().isMacroID())
+    return;
+
+  const Expr *Arg = Call->getArg(0)->IgnoreParenImpCasts();
+  // Arguments which are macros are ignored.
+  if (Arg->getBeginLoc().isMacroID())
+    return;
+
+  // We first handle the cases of literal zero (both float and integer).
+  if (Result.Nodes.getNodeAs<Stmt>("zero")) {
+    diag(Call->getBeginLoc(),
+         "use ZeroDuration() for zero-length time intervals")
+        << FixItHint::CreateReplacement(Call->getSourceRange(),
+                                        "absl::ZeroDuration()");
+    return;
+  }
+
+  const auto *CallDecl = Result.Nodes.getNodeAs<FunctionDecl>("call_decl");
+  llvm::Optional<DurationScale> MaybeScale =
+      getScaleForFactory(CallDecl->getName());
+  if (!MaybeScale)
+    return;
+
+  DurationScale Scale = *MaybeScale;
+  const Expr *Remainder;
+  llvm::Optional<DurationScale> NewScale;
+
+  // We next handle the cases of multiplication and division.
+  if (const auto *MultBinOp =
+          Result.Nodes.getNodeAs<BinaryOperator>("mult_binop")) {
+    // For multiplication, we need to look at both operands, and consider the
+    // cases where a user is multiplying by something such as 1e-3.
+
+    // First check the LHS
+    const auto *IntLit = llvm::dyn_cast<IntegerLiteral>(MultBinOp->getLHS());
+    const auto *FloatLit = llvm::dyn_cast<FloatingLiteral>(MultBinOp->getLHS());
+    if (IntLit || FloatLit) {
+      NewScale = GetNewScale(Scale, GetValue(IntLit, FloatLit));
+      if (NewScale)
+        Remainder = MultBinOp->getRHS();
+    }
+
+    // If we weren't able to scale based on the LHS, check the RHS
+    if (!NewScale) {
+      IntLit = llvm::dyn_cast<IntegerLiteral>(MultBinOp->getRHS());
+      FloatLit = llvm::dyn_cast<FloatingLiteral>(MultBinOp->getRHS());
+      if (IntLit || FloatLit) {
+        NewScale = GetNewScale(Scale, GetValue(IntLit, FloatLit));
+        if (NewScale)
+          Remainder = MultBinOp->getLHS();
+      }
+    }
+  } else if (const auto *DivBinOp =
+                 Result.Nodes.getNodeAs<BinaryOperator>("div_binop")) {
+    // We next handle division.
+    // For division, we only check the RHS.
+    const auto *FloatLit = llvm::dyn_cast<FloatingLiteral>(DivBinOp->getRHS());
+
+    llvm::Optional<DurationScale> NewScale =
+        GetNewScale(Scale, 1.0 / FloatLit->getValueAsApproximateDouble());
+    if (NewScale) {
+      const Expr *Remainder = DivBinOp->getLHS();
+
+      // We've found an appropriate scaling factor and the new scale, so output
+      // the relevant fix.
+      diag(Call->getBeginLoc(), "internal duration scaling can be removed")
+          << FixItHint::CreateReplacement(
+                 Call->getSourceRange(),
+                 (llvm::Twine(GetFactoryForScale(*NewScale)) + "(" +
+                  tooling::fixit::getText(*Remainder, *Result.Context) + ")")
+                     .str());
+    }
+  }
+
+  if (NewScale) {
+    assert(Remainder && "No remainder found");
+    // We've found an appropriate scaling factor and the new scale, so output
+    // the relevant fix.
+    diag(Call->getBeginLoc(), "internal duration scaling can be removed")
+        << FixItHint::CreateReplacement(
+               Call->getSourceRange(),
+               (llvm::Twine(GetFactoryForScale(*NewScale)) + "(" +
+                tooling::fixit::getText(*Remainder, *Result.Context) + ")")
+                   .str());
+  }
+  return;
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.h?rev=347163&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.h Sun Nov 18 08:41:06 2018
@@ -0,0 +1,38 @@
+//===--- DurationFactoryScaleCheck.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_ABSEIL_DURATIONFACTORYSCALECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONFACTORYSCALECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This check finds cases where the incorrect `Duration` factory function is
+/// being used by looking for scaling constants inside the factory argument
+/// and suggesting a more appropriate factory.  It also looks for the special
+/// case of zero and suggests `ZeroDuration()`.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-factory-scale.html
+class DurationFactoryScaleCheck : public ClangTidyCheck {
+public:
+  DurationFactoryScaleCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONFACTORYSCALECHECK_H

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=347163&r1=347162&r2=347163&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Sun Nov 18 08:41:06 2018
@@ -81,6 +81,12 @@ Improvements to clang-tidy
   ``absl::Duration`` factory functions are called when the more-efficient
   integer versions could be used instead.
 
+- New :doc:`abseil-duration-factory-scale
+  <clang-tidy/checks/abseil-duration-factory-scale>` check.
+
+  Checks for cases where arguments to ``absl::Duration`` factory functions are
+  scaled internally and could be changed to a different factory function.
+
 - New :doc:`abseil-faster-strsplit-delimiter
   <clang-tidy/checks/abseil-faster-strsplit-delimiter>` check.
 

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-factory-scale.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-factory-scale.rst?rev=347163&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-factory-scale.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-factory-scale.rst Sun Nov 18 08:41:06 2018
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - abseil-duration-factory-scale
+
+abseil-duration-factory-scale
+=============================
+
+Checks for cases where arguments to ``absl::Duration`` factory functions are
+scaled internally and could be changed to a different factory function. This
+check also looks for arguements with a zero value and suggests using
+``absl::ZeroDuration()`` instead.
+
+Examples:
+
+.. code-block:: c++
+
+  // Original - Internal multiplication.
+  int x;
+  absl::Duration d = absl::Seconds(60 * x);
+
+  // Suggested - Use absl::Minutes instead.
+  absl::Duration d = absl::Minutes(x);
+
+
+  // Original - Internal division.
+  int y;
+  absl::Duration d = absl::Milliseconds(y / 1000.);
+
+  // Suggested - Use absl:::Seconds instead.
+  absl::Duration d = absl::Seconds(y);
+
+
+  // Original - Zero-value argument.
+  absl::Duration d = absl::Hours(0);
+
+  // Suggested = Use absl::ZeroDuration instead
+  absl::Duration d = absl::ZeroDuration();

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=347163&r1=347162&r2=347163&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Sun Nov 18 08:41:06 2018
@@ -6,6 +6,7 @@ Clang-Tidy Checks
 .. toctree::
    abseil-duration-division
    abseil-duration-factory-float
+   abseil-duration-factory-scale
    abseil-faster-strsplit-delimiter
    abseil-no-internal-dependencies
    abseil-no-namespace

Added: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp?rev=347163&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp Sun Nov 18 08:41:06 2018
@@ -0,0 +1,130 @@
+// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+  Duration NAME(float n);                         \
+  Duration NAME(double n);                        \
+  template <typename T>                           \
+  Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+}  // namespace absl
+
+void ScaleTest() {
+  absl::Duration d;
+
+  // Zeroes
+  d = absl::Hours(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Minutes(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(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Milliseconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Microseconds(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use ZeroDuration() for zero-length time intervals [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::ZeroDuration();
+  d = absl::Nanoseconds(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(0.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(0x0.000001p-126f);
+  // 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);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+  d = absl::Seconds(60 * 30);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+
+  // Try a few more exotic multiplications
+  d = absl::Seconds(60 * 30 * 60);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(60 * 30);
+  d = absl::Seconds(1e-3 * 30);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Milliseconds(30);
+  d = absl::Milliseconds(30 * 1000);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Seconds(30);
+  d = absl::Milliseconds(30 * 0.001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(30);
+
+  // Multiple steps
+  d = absl::Seconds(5 * 3600);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Hours(5);
+  d = absl::Microseconds(5 * 1e6);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Seconds(5);
+  d = absl::Seconds(5 * 1e-6);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(5);
+  d = absl::Microseconds(5 * 1000000);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Seconds(5);
+
+  // Division
+  d = absl::Hours(30 / 60.);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Minutes(30);
+  d = absl::Seconds(30 / 1000.);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Milliseconds(30);
+  d = absl::Milliseconds(30 / 1e3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(30);
+  d = absl::Seconds(30 / 1e6);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: internal duration scaling can be removed [abseil-duration-factory-scale]
+  // CHECK-FIXES: absl::Microseconds(30);
+
+  // None of these should trigger the check
+  d = absl::Seconds(60);
+  d = absl::Seconds(60 + 30);
+  d = absl::Seconds(60 - 30);
+  d = absl::Seconds(50 * 30);
+  d = absl::Hours(60 * 60);
+  d = absl::Nanoseconds(1e-3 * 30);
+  d = absl::Seconds(1000 / 30);
+  // We don't support division by integers, which could cause rounding
+  d = absl::Seconds(10 / 1000);
+  d = absl::Seconds(30 / 50);
+
+#define EXPRESSION 30 * 60
+  d = absl::Seconds(EXPRESSION);
+#undef EXPRESSION
+
+// This should not trigger
+#define HOURS(x) absl::Minutes(60 * x)
+  d = HOURS(40);
+#undef HOURS
+}




More information about the cfe-commits mailing list