[clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[astrelni]

Summary

Introduce a new check to upgrade user code based on upcoming API breaking changes to absl::Duration.

The check finds calls to arithmetic operators and factory functions for absl::Duration that rely on an implicit user defined conversion to int64_t. These cases will no longer compile after proposed changes are released. Suggested fixes explicitly cast the argument int64_t.

[Eugene.Zelenko]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

22

Please add check for C++. See other Abseil checks code as example.

136

Please don't use auto, since return type is not obvious.

docs/ReleaseNotes.rst

70

Please sort new checks list alphabetically.

[astrelni]

Reply to comments: add check for language being c++, explicitly name type in declaration, alphabetize release notes.

[JonasToth]

Hi astrelni,

my 2cents.
Please upload the patch will full context (i believe diff -U 99999, but check the man pages if that doesn't work :D)

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

33

Please add types in your examples, to make clear what actually happens.

119

What happens on invalid ranges? Are they considered equal, or is it forbidden to pass them in?

124

Please add a test-case where the Node.getSourceRange() is a macro, if not already existing. I believe that is currently missing.

135

ast_matchers:: is not necessary, as there is a using ... at the top of the file.

159

You could ellide these braces, but I feel that this matching code be merged into one matcher with equalsBoundNode() (see ASTMatcher reference).

165

my personal preference would be removing the builtin-array and add two << FixitHint call to the diag.

clang-tidy/abseil/UpgradeDurationConversionsCheck.h

20

please mark code construct with ' in comments.

docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst

44

please remove the last empty line

docs/clang-tidy/checks/list.rst

13

spurious changes in this file.

test/clang-tidy/abseil-upgrade-duration-conversions.cpp

143

what about non-type template parameters? Do they need consideration for the check as well?
If i am not mistaken floats are allowed in newwer standards as well.

[astrelni]

Thanks for the feedback so far.

Reply to review comments.

• Improve comments.
• Fix matcher to check for invalid source range.
• Improve test coverage for templates and macros.

[astrelni]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

119

Good point. I think this is a non-issue with the way the code is set up now, but better make it explicitly work :). We only care about valid source ranges. I've updated the name of the matcher and added a check here for the Node's source range and another in the matcher below to return false early.

124

Yes, thank you, that was missing. Added a few macro + template interactions.

159

Started with removing braces.

Sorry I had a look at equalsBoundNode(), but couldn't see exactly what you meant. Could you please elaborate about the merging?

test/clang-tidy/abseil-upgrade-duration-conversions.cpp

143

IIUC non-type template parameters should be no different for this check. This particular case is to make sure explicit specializations are treated differently from instantiations and get fix-it hints.

[JonasToth]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

159

i do not use that matcher on a daily basis, so take it with a grain of salt, please :)

prototype:

match(unless(allOf(isInTemplateInstantiation(equalsBoundNode("arg"))),
                                 expr(isInstantiationOfDependentExpr(equalsBoundNode("call")))))

You could do that matching in the registerMatchers even. The equalsBoundNode is suitable to connect different parts of a complexer matcher.
If you want, you can explore it. Slight simplification would be desirable here, but if it's not feasable the current form works too.

[astrelni]

Combine template instantiation matching into the other matcher registration.

[astrelni]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

159

Got it work out in registerMatchers and without needing equalsBoundNode, thanks for prodding me in that direction. I tried it initially when writing the check and can't remember why I didn't get it to work last time.

[JonasToth]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

168

Minor, but you can move diag(...) out the if condition like this:

auto Diag = diag(...);

and remove the duplication in the diag creation. The << works the same on the variable.

test/clang-tidy/abseil-upgrade-duration-conversions.cpp

54

Please add tests where the RHS is a complex arithmetic expression.

d = number1 + number2 + number3;

And the like, that trigger the warning and please ensure, the provided transformation is correct.

143

Please test in that case that the behaviour is as expected.

[aaron.ballman]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

152–153

How about: "implicit conversion to 'int64_t' is deprecated in this context; use an explicit cast instead"

docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst

17

floating point -> floating-point

23

compile error -> a diagnostic

43

floating point -> floating-point

[astrelni]

Reply to review comments:

• minor code reorder
• improve test coverage

test/clang-tidy/abseil-upgrade-duration-conversions.cpp

143

Added some cases. I hope I interpreted correctly that "in that case" meant for template specializations / instantiations for non type template parameters.

[astrelni]

Reply to comments:

• Change diagnostic message
• Update documentation

docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst

23

Tried rewording for clarity, let me know what you think. It doesn't seem like diagnostic is what's needed here as its talking about the upcoming API changes to Abseil and not the affects of this check.

[astrelni]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

152–153

Forgot to say thanks, this is much better :)

[alexfh]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

34–37

The hasAncestor and hasDescendant matchers frequently have non-trivial performance implications. Especially the latter, especially when called on a large number of nodes with a large number of transitive children (this obviously depends on the code being analyzed). I don't know how likely is this to cause problems here (the matcher seems to only be used when the check is about to issue a diagnostic), but it's always good to be aware of the possible issues.

Frequently there's a more efficient (and easier to understand) alternative. For example, instead of trying to evaluate certain conditions while traversing the code (which may require a large number of lookups into relatively distant parts of the AST), it's sometimes more efficient to split the analysis into two or more stages. During the AST traversal (i.e. when AST matchers run) the check could collect some raw information about all potentially problematic places in the code and then (for example, in onEndOfTranslationUnit) analyze the collected information together in a second stage. This works best if there's a way to arrange the data gathered on the first pass such that the second pass can efficiently look up necessary information.

137–142

Use hasAnyName instead.

[astrelni]

Updated filtering of template instantiations to not use potentially costly hasDescendent matcher.

[astrelni]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

34–37

Thanks, gave it a try, what do you think of this?

[alexfh]

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

34–37

From a cursory look this should be less likely to be sloooow. In any case, makes sense to profile the check on a number of large files (at least against other checks, using clang-tidy's -enable-check-profile option).

44

hasAnyName, please. It's more efficient. A few more instances below.

[astrelni]

Fix to use hasAnyName everywhere

[astrelni]

Fix incorrect uploaded diff.

[alexfh]

LG with one more comment. Please address other reviewers' comments though.

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

154

MatchedTemplateLocations can be a std::set<SourceLocation> or a std::unordered_set<unsigned> filled with SourceLocation::getRawEncoding(). Same for MatchedInstantiationLocations, I suppose.

[astrelni]

Sorry for the long delay.

I've reworked the template instantiation stuff a little bit yet again. Going to still come back and comment with results of profiling but I think now this shouldn't be much slower than if the template instantiations were filtered out with the matchers.

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

154

Ah thank you, I didn't look hard enough after not seeing a std::hash specialization for SourceLocation. In that case we don't need the onEndOfTranslationUnit or MatchedInstantiationLocations so those are now gone. I added a test case for when a template is used in source before the body since I wasn't 100% sure that the template would be traversed before the instantiation.

[hokein]

+ @hwright who have implemented a bunch of absl-duration-* checks, you might be interested in this as well.

[EricWF]

Committed as r348633.

clang-tidy/abseil/AbseilTidyModule.cpp

 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "UpgradeDurationConversionsCheck.h"
 
 namespace clang {
 namespace tidy {

         "abseil-str-cat-append");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
         "abseil-string-find-startswith");
+    CheckFactories.registerCheck<UpgradeDurationConversionsCheck>(
+        "abseil-upgrade-duration-conversions");
   }
 };
 

clang-tidy/abseil/CMakeLists.txt

   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  UpgradeDurationConversionsCheck.cpp
 
   LINK_LIBS
   clangAST

clang-tidy/abseil/UpgradeDurationConversionsCheck.h

+//===--- UpgradeDurationConversionsCheck.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_UPGRADEDURATIONCONVERSIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_UPGRADEDURATIONCONVERSIONSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds deprecated uses of absl::Duration arithmetic operators and factories.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-upgrade-duration-conversions.html
+class UpgradeDurationConversionsCheck : public ClangTidyCheck {
+public:
+  UpgradeDurationConversionsCheck(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_UPGRADEDURATIONCONVERSIONSCHECK_H

clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

+//===--- UpgradeDurationConversionsCheck.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 "UpgradeDurationConversionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void UpgradeDurationConversionsCheck::registerMatchers(
+    ast_matchers::MatchFinder *Finder) {
+  // For the arithmetic calls, we match only the uses of the templated operators
+  // where the template parameter is not a built-in type. This means the
+  // instantiation makes use of an available user defined conversion to int64_t.
+  //
+  // The implementation of these templates will be updated to fail SFINAE for
+  // non-integral types. We match them to suggest an explicit cast.
+
+  // a *= b; a /= b;
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          argumentCountIs(2),
+          hasArgument(
+              0, expr(hasType(cxxRecordDecl(hasName("::absl::Duration"))))),
+          hasArgument(1, expr().bind("arg")),
+          callee(functionDecl(
+              hasParent(functionTemplateDecl()),
+              unless(hasTemplateArgument(0, refersToType(builtinType()))),
+              anyOf(hasName("operator*="), hasName("operator/=")))))
+          .bind("call"),
+      this);
+
+  // a.operator*=(b); a.operator/=(b);
+  Finder->addMatcher(
+      cxxMemberCallExpr(
+          callee(cxxMethodDecl(
+              ofClass(cxxRecordDecl(hasName("::absl::Duration"))),
+              hasParent(functionTemplateDecl()),
+              unless(hasTemplateArgument(0, refersToType(builtinType()))),
+              anyOf(hasName("operator*="), hasName("operator/=")))),
+          argumentCountIs(1), hasArgument(0, expr().bind("arg")))
+          .bind("call"),
+      this);
+
+  // a * b; a / b; operator*(a, b); operator/(a, b); where T is second arg
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
+                   hasParent(functionTemplateDecl()),
+                   unless(hasTemplateArgument(0, refersToType(builtinType()))),
+                   anyOf(hasName("::absl::operator*"),
+                         hasName("::absl::operator/")))),
+               argumentCountIs(2),
+               hasArgument(0, expr(hasType(
+                                  cxxRecordDecl(hasName("::absl::Duration"))))),
+               hasArgument(1, expr().bind("arg")))
+          .bind("call"),
+      this);
+
+  // a * b; operator*(a, b); where T is first arg
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(
+                   hasParent(functionTemplateDecl()),
+                   unless(hasTemplateArgument(0, refersToType(builtinType()))),
+                   hasName("::absl::operator*"))),
+               argumentCountIs(2), hasArgument(0, expr().bind("arg")),
+               hasArgument(1, expr(hasType(
+                                  cxxRecordDecl(hasName("::absl::Duration"))))))
+          .bind("call"),
+      this);
+
+  // For the factory functions, we match only the non-templated overloads that
+  // take an int64_t parameter. Within these calls, we care about implicit
+  // casts through a user defined conversions to int64_t.
+  //
+  // The factory functions will be updated to be templated and SFINAE on whether
+  // the template parameter is an integral type. This complements the already
+  // existing templated overloads that only accept floating point types.
+
+  // absl::Nanoseconds()
+  // absl::Microseconds()
+  // absl::Milliseconds()
+  // absl::Seconds()
+  // absl::Minutes()
+  // absl::Hours()
+  Finder->addMatcher(
+      implicitCastExpr(
+          anyOf(hasCastKind(CK_UserDefinedConversion),
+                has(implicitCastExpr(hasCastKind(CK_UserDefinedConversion)))),
+          hasParent(callExpr(callee(functionDecl(
+                                 anyOf(hasName("::absl::Nanoseconds"),
+                                       hasName("::absl::Microseconds"),
+                                       hasName("::absl::Milliseconds"),
+                                       hasName("::absl::Seconds"),
+                                       hasName("::absl::Minutes"),
+                                       hasName("::absl::Hours")),
+                                 unless(hasParent(functionTemplateDecl())))),
+                             hasArgument(0, expr().bind("arg")))
+                        .bind("call"))),
+      this);
+}
+
+namespace {
+
+AST_MATCHER_P(Expr, hasSourceRange, SourceRange, Range) {
+  return Node.getSourceRange() == Range;
+}
+
+AST_MATCHER(Expr, isInstantiationOfDependentExpr) {
+  auto HasMatchingDependentDescendant = hasDescendant(
+      expr(hasSourceRange(Node.getSourceRange()), isInstantiationDependent()));
+  return expr(anyOf(hasAncestor(
+                        functionTemplateDecl(HasMatchingDependentDescendant)),
+                    hasAncestor(
+                        classTemplateDecl(HasMatchingDependentDescendant))))
+      .matches(Node, Finder, Builder);
+}
+
+} // namespace
+
+void UpgradeDurationConversionsCheck::check(
+    const ast_matchers::MatchFinder::MatchResult &Result) {
+  const auto *ArgExpr = Result.Nodes.getNodeAs<Expr>("arg");
+  llvm::StringRef Message = "perform explicit cast on expression getting "
+                            "implicitly converted to int64_t";
+
+  auto SourceRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(ArgExpr->getSourceRange()),
+      *Result.SourceManager, Result.Context->getLangOpts());
+  if (SourceRange.isInvalid()) {
+    // The invalid source range likely means we are inside a macro body. Warn
+    // but do not try to suggest a fix.
+    diag(ArgExpr->getBeginLoc(), Message);
+    return;
+  }
+
+  // If we are inside a template instantiation, we warn if the call expression
+  // in the original template is dependent on the instantiation, (i.e. depends
+  // on template parameters). This means the expression in the template does not
+  // trigger without the instantiation. Do not suggest a fix in this case, since
+  // a manual fix is likely needed.
+  if (!match(isInTemplateInstantiation(), *ArgExpr, *Result.Context).empty()) {
+    const auto *CallExpr = Result.Nodes.getNodeAs<Expr>("call");
+    if (!match(expr(isInstantiationOfDependentExpr()), *CallExpr,
+               *Result.Context)
+             .empty()) {
+      diag(ArgExpr->getBeginLoc(), Message);
+    }
+    return;
+  }
+
+  FixItHint Fixes[] = {FixItHint::CreateInsertion(SourceRange.getBegin(),
+                                                  "static_cast<int64_t>("),
+                       FixItHint::CreateInsertion(SourceRange.getEnd(), ")")};
+  diag(ArgExpr->getBeginLoc(), Message) << Fixes;
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

 Improvements to clang-tidy
 --------------------------
 
+- New :doc:`abseil-upgrade-duration-conversions
+  <clang-tidy/checks/abseil-upgrade-duration-conversions>` check.
+
+  Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+  argument needs an explicit cast to continue compiling after upcoming API
+  changes.
+
 - New :doc:`abseil-duration-division
   <clang-tidy/checks/abseil-duration-division>` check.
 

docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst

+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===================================
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic<float>`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+The operators and factories will be updated to only accept arithmetic types to
+prevent unintended behavior. Passing an argument of class type will result in
+compile error, even if the type is implicitly convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+  std::atomic<int> a;
+  absl::Duration d = absl::Milliseconds(a);
+  d *= a;
+
+becomes
+
+.. code-block:: c++
+
+  std::atomic<int> a;
+  absl::Duration d = absl::Milliseconds(static_cast<int64_t>(a));
+  d *= static_cast<int64_t>(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating point type.
+

docs/clang-tidy/checks/list.rst

    abseil-no-internal-dependencies
    abseil-no-namespace
    abseil-redundant-strcat-calls
+   abseil-str-cat-append
    abseil-string-find-startswith
-   abseil-str-cat-append
+   abseil-upgrade-duration-conversions
    android-cloexec-accept
    android-cloexec-accept4
    android-cloexec-creat

    hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) <hicpp-special-member-functions>
    hicpp-static-assert (redirects to misc-static-assert) <hicpp-static-assert>
    hicpp-undelegated-constructor (redirects to bugprone-undelegated-constructor) <hicpp-undelegated-constructor>
+   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) <hicpp-uppercase-literal-suffix>
    hicpp-use-auto (redirects to modernize-use-auto) <hicpp-use-auto>
    hicpp-use-emplace (redirects to modernize-use-emplace) <hicpp-use-emplace>
    hicpp-use-equals-default (redirects to modernize-use-equals-default) <hicpp-use-equals-default>

    hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
    hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
    hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
-   hicpp-uppercase-literal-suffix (redirects to readability-uppercase-literal-suffix) <hicpp-uppercase-literal-suffix>
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment

test/clang-tidy/abseil-upgrade-duration-conversions.cpp

+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+  Duration &operator*=(int64_t r);
+  Duration &operator*=(float r);
+  Duration &operator*=(double r);
+  template <typename T> Duration &operator*=(T r);
+
+  Duration &operator/=(int64_t r);
+  Duration &operator/=(float r);
+  Duration &operator/=(double r);
+  template <typename T> Duration &operator/=(T r);
+};
+
+template <typename T> Duration operator*(Duration lhs, T rhs);
+template <typename T> Duration operator*(T lhs, Duration rhs);
+template <typename T> Duration operator/(Duration lhs, T rhs);
+
+constexpr Duration Nanoseconds(int64_t n);
+constexpr Duration Microseconds(int64_t n);
+constexpr Duration Milliseconds(int64_t n);
+constexpr Duration Seconds(int64_t n);
+constexpr Duration Minutes(int64_t n);
+constexpr Duration Hours(int64_t n);
+
+template <typename T> struct EnableIfFloatImpl {};
+template <> struct EnableIfFloatImpl<float> { typedef int Type; };
+template <> struct EnableIfFloatImpl<double> { typedef int Type; };
+template <> struct EnableIfFloatImpl<long double> { typedef int Type; };
+template <typename T> using EnableIfFloat = typename EnableIfFloatImpl<T>::Type;
+
+template <typename T, EnableIfFloat<T> = 0> Duration Nanoseconds(T n);
+template <typename T, EnableIfFloat<T> = 0> Duration Microseconds(T n);
+template <typename T, EnableIfFloat<T> = 0> Duration Milliseconds(T n);
+template <typename T, EnableIfFloat<T> = 0> Duration Seconds(T n);
+template <typename T, EnableIfFloat<T> = 0> Duration Minutes(T n);
+template <typename T, EnableIfFloat<T> = 0> Duration Hours(T n);
+
+} // namespace absl
+
+template <typename T> struct ConvertibleTo {
+  operator T() const;
+};
+
+void arithmeticOperatorBasicPositive() {
+  absl::Duration d;
+  d *= ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d *= static_cast<int64_t>(ConvertibleTo<int64_t>());
+  d /= ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d /= static_cast<int64_t>(ConvertibleTo<int64_t>());
+  d = ConvertibleTo<int64_t>() * d;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = static_cast<int64_t>(ConvertibleTo<int64_t>()) * d;
+  d = d * ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = d * static_cast<int64_t>(ConvertibleTo<int64_t>());
+  d = d / ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = d / static_cast<int64_t>(ConvertibleTo<int64_t>());
+  d.operator*=(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d.operator*=(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  d.operator/=(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d.operator/=(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  d = operator*(ConvertibleTo<int64_t>(), d);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = operator*(static_cast<int64_t>(ConvertibleTo<int64_t>()), d);
+  d = operator*(d, ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = operator*(d, static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  d = operator/(d, ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = operator/(d, static_cast<int64_t>(ConvertibleTo<int64_t>()));
+}
+
+void arithmeticOperatorBasicNegative() {
+  absl::Duration d;
+  d *= char{1};
+  d *= 1;
+  d *= int64_t{1};
+  d *= 1.0f;
+  d *= 1.0;
+  d *= 1.0l;
+  d /= char{1};
+  d /= 1;
+  d /= int64_t{1};
+  d /= 1.0f;
+  d /= 1.0;
+  d /= 1.0l;
+  d = d * char{1};
+  d = d * 1;
+  d = d * int64_t{1};
+  d = d * 1.0f;
+  d = d * 1.0;
+  d = d * 1.0l;
+  d = char{1} * d;
+  d = 1 * d;
+  d = int64_t{1} * d;
+  d = 1.0f * d;
+  d = 1.0 * d;
+  d = 1.0l * d;
+  d = d / char{1};
+  d = d / 1;
+  d = d / int64_t{1};
+  d = d / 1.0f;
+  d = d / 1.0;
+  d = d / 1.0l;
+
+  d *= static_cast<int>(ConvertibleTo<int>());
+  d *= (int)ConvertibleTo<int>();
+  d *= int(ConvertibleTo<int>());
+  d /= static_cast<int>(ConvertibleTo<int>());
+  d /= (int)ConvertibleTo<int>();
+  d /= int(ConvertibleTo<int>());
+  d = static_cast<int>(ConvertibleTo<int>()) * d;
+  d = (int)ConvertibleTo<int>() * d;
+  d = int(ConvertibleTo<int>()) * d;
+  d = d * static_cast<int>(ConvertibleTo<int>());
+  d = d * (int)ConvertibleTo<int>();
+  d = d * int(ConvertibleTo<int>());
+  d = d / static_cast<int>(ConvertibleTo<int>());
+  d = d / (int)ConvertibleTo<int>();
+  d = d / int(ConvertibleTo<int>());
+
+  d *= 1 + ConvertibleTo<int>();
+  d /= 1 + ConvertibleTo<int>();
+  d = (1 + ConvertibleTo<int>()) * d;
+  d = d * (1 + ConvertibleTo<int>());
+  d = d / (1 + ConvertibleTo<int>());
+}
+
+template <typename T> void templateForOpsSpecialization(T) {}
+template <>
+void templateForOpsSpecialization<absl::Duration>(absl::Duration d) {
+  d *= ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d *= static_cast<int64_t>(ConvertibleTo<int64_t>());
+}
+
+template <typename T> void templateOpsFix() {
+  absl::Duration d;
+  d *= ConvertibleTo<int64_t>();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d *= static_cast<int64_t>(ConvertibleTo<int64_t>());
+}
+
+template <typename T, typename U> void templateOpsWarnOnly(T t, U u) {
+  t *= u;
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  absl::Duration d;
+  d *= u;
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+}
+
+template <typename T> struct TemplateTypeOpsWarnOnly {
+  void memberA(T t) {
+    d *= t;
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  }
+  template <typename U, typename V> void memberB(U u, V v) {
+    u *= v;
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: perform explicit cast on expression getting implicitly converted to int64_t
+    d *= v;
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  }
+
+  absl::Duration d;
+};
+
+void arithmeticOperatorsInTemplates() {
+  templateForOpsSpecialization(5);
+  templateForOpsSpecialization(absl::Duration());
+  templateOpsFix<int>();
+  templateOpsWarnOnly(absl::Duration(), ConvertibleTo<int>());
+  TemplateTypeOpsWarnOnly<ConvertibleTo<int>> t;
+  t.memberA(ConvertibleTo<int>());
+  t.memberB(absl::Duration(), ConvertibleTo<int>());
+}
+
+#define FUNCTION_MACRO(x) x
+#define CONVERTIBLE_TMP ConvertibleTo<int>()
+#define ONLY_WARN_INSIDE_MACRO_ARITHMETIC_OP d *= ConvertibleTo<int>()
+
+void arithmeticOperatorsInMacros() {
+  absl::Duration d;
+  d = FUNCTION_MACRO(d * ConvertibleTo<int>());
+  // CHECK-MESSAGES: [[@LINE-1]]:26: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d = FUNCTION_MACRO(d * static_cast<int64_t>(ConvertibleTo<int>()));
+  d *= FUNCTION_MACRO(ConvertibleTo<int>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d *= static_cast<int64_t>(FUNCTION_MACRO(ConvertibleTo<int>()));
+  d *= CONVERTIBLE_TMP;
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: d *= static_cast<int64_t>(CONVERTIBLE_TMP);
+  ONLY_WARN_INSIDE_MACRO_ARITHMETIC_OP;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: perform explicit cast on expression getting implicitly converted to int64_t
+}
+
+void factoryFunctionPositive() {
+  // User defined conversion:
+  (void)absl::Nanoseconds(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  (void)absl::Microseconds(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Microseconds(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  (void)absl::Milliseconds(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Milliseconds(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  (void)absl::Seconds(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Seconds(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  (void)absl::Minutes(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Minutes(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+  (void)absl::Hours(ConvertibleTo<int64_t>());
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Hours(static_cast<int64_t>(ConvertibleTo<int64_t>()));
+
+  // User defined conversion to integral type, followed by built-in conversion:
+  (void)absl::Nanoseconds(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(ConvertibleTo<char>()));
+  (void)absl::Microseconds(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Microseconds(static_cast<int64_t>(ConvertibleTo<char>()));
+  (void)absl::Milliseconds(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Milliseconds(static_cast<int64_t>(ConvertibleTo<char>()));
+  (void)absl::Seconds(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Seconds(static_cast<int64_t>(ConvertibleTo<char>()));
+  (void)absl::Minutes(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Minutes(static_cast<int64_t>(ConvertibleTo<char>()));
+  (void)absl::Hours(ConvertibleTo<char>());
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Hours(static_cast<int64_t>(ConvertibleTo<char>()));
+
+  // User defined conversion to floating point type, followed by built-in conversion:
+  (void)absl::Nanoseconds(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(ConvertibleTo<float>()));
+  (void)absl::Microseconds(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Microseconds(static_cast<int64_t>(ConvertibleTo<float>()));
+  (void)absl::Milliseconds(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Milliseconds(static_cast<int64_t>(ConvertibleTo<float>()));
+  (void)absl::Seconds(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Seconds(static_cast<int64_t>(ConvertibleTo<float>()));
+  (void)absl::Minutes(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Minutes(static_cast<int64_t>(ConvertibleTo<float>()));
+  (void)absl::Hours(ConvertibleTo<float>());
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Hours(static_cast<int64_t>(ConvertibleTo<float>()));
+}
+
+void factoryFunctionNegative() {
+  (void)absl::Nanoseconds(char{1});
+  (void)absl::Nanoseconds(1);
+  (void)absl::Nanoseconds(int64_t{1});
+  (void)absl::Nanoseconds(1.0f);
+  (void)absl::Microseconds(char{1});
+  (void)absl::Microseconds(1);
+  (void)absl::Microseconds(int64_t{1});
+  (void)absl::Microseconds(1.0f);
+  (void)absl::Milliseconds(char{1});
+  (void)absl::Milliseconds(1);
+  (void)absl::Milliseconds(int64_t{1});
+  (void)absl::Milliseconds(1.0f);
+  (void)absl::Seconds(char{1});
+  (void)absl::Seconds(1);
+  (void)absl::Seconds(int64_t{1});
+  (void)absl::Seconds(1.0f);
+  (void)absl::Minutes(char{1});
+  (void)absl::Minutes(1);
+  (void)absl::Minutes(int64_t{1});
+  (void)absl::Minutes(1.0f);
+  (void)absl::Hours(char{1});
+  (void)absl::Hours(1);
+  (void)absl::Hours(int64_t{1});
+  (void)absl::Hours(1.0f);
+
+  (void)absl::Nanoseconds(static_cast<int>(ConvertibleTo<int>()));
+  (void)absl::Microseconds(static_cast<int>(ConvertibleTo<int>()));
+  (void)absl::Milliseconds(static_cast<int>(ConvertibleTo<int>()));
+  (void)absl::Seconds(static_cast<int>(ConvertibleTo<int>()));
+  (void)absl::Minutes(static_cast<int>(ConvertibleTo<int>()));
+  (void)absl::Hours(static_cast<int>(ConvertibleTo<int>()));
+}
+
+template <typename T> void templateForFactorySpecialization(T) {}
+template <> void templateForFactorySpecialization<ConvertibleTo<int>>(ConvertibleTo<int> c) {
+  (void)absl::Nanoseconds(c);
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(c));
+}
+
+template <typename T> void templateFactoryFix() {
+  (void)absl::Nanoseconds(ConvertibleTo<int>());
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(ConvertibleTo<int>()));
+}
+
+template <typename T> void templateFactoryWarnOnly(T t) {
+  (void)absl::Nanoseconds(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+}
+
+template <typename T> struct TemplateTypeFactoryWarnOnly {
+  void memberA(T t) {
+    (void)absl::Nanoseconds(t);
+    // CHECK-MESSAGES: [[@LINE-1]]:29: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  }
+  template <typename U> void memberB(U u) {
+    (void)absl::Nanoseconds(u);
+    // CHECK-MESSAGES: [[@LINE-1]]:29: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  }
+};
+
+void factoryInTemplates() {
+  templateForFactorySpecialization(5);
+  templateForFactorySpecialization(ConvertibleTo<int>());
+  templateFactoryFix<int>();
+  templateFactoryWarnOnly(ConvertibleTo<int>());
+  TemplateTypeFactoryWarnOnly<ConvertibleTo<int>> t;
+  t.memberA(ConvertibleTo<int>());
+  t.memberB(ConvertibleTo<int>());
+}
+
+#define ONLY_WARN_INSIDE_MACRO_FACTORY                                         \
+  (void)absl::Nanoseconds(ConvertibleTo<int>())
+
+void factoryInMacros() {
+  (void)absl::Nanoseconds(FUNCTION_MACRO(ConvertibleTo<int>()));
+  // CHECK-MESSAGES: [[@LINE-1]]:42: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(FUNCTION_MACRO(ConvertibleTo<int>())));
+  (void)absl::Nanoseconds(CONVERTIBLE_TMP);
+  // CHECK-MESSAGES: [[@LINE-1]]:27: warning: perform explicit cast on expression getting implicitly converted to int64_t
+  // CHECK-FIXES: (void)absl::Nanoseconds(static_cast<int64_t>(CONVERTIBLE_TMP))
+  ONLY_WARN_INSIDE_MACRO_FACTORY;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: perform explicit cast on expression getting implicitly converted to int64_t
+}
+