[clang-tidy] Abseil: integral division of Duration check

This check is an abseil specific test that tests to ensure users utilize abseil specific floating point division when trying to divide with abseil duration types.

Patch by Deanna Garcia!

llvm-svn: 340038

clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp

@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "DurationDivisionCheck.h"
 #include "StringFindStartswithCheck.h"
 
 namespace clang {
@@ -19,6 +20,8 @@ namespace abseil {
 class AbseilModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<DurationDivisionCheck>(
+        "abseil-duration-division");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
         "abseil-string-find-startswith");
   }

clang-tools-extra/clang-tidy/abseil/CMakeLists.txt

@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
+  DurationDivisionCheck.cpp
   StringFindStartswithCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.cpp

@@ -0,0 +1,59 @@
+//===--- DurationDivisionCheck.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 "DurationDivisionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+using namespace clang::ast_matchers;
+
+void DurationDivisionCheck::registerMatchers(MatchFinder *finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  const auto DurationExpr =
+      expr(hasType(cxxRecordDecl(hasName("::absl::Duration"))));
+  finder->addMatcher(
+      implicitCastExpr(
+          hasSourceExpression(ignoringParenCasts(
+              cxxOperatorCallExpr(hasOverloadedOperatorName("/"),
+                                  hasArgument(0, DurationExpr),
+                                  hasArgument(1, DurationExpr))
+                  .bind("OpCall"))),
+          hasImplicitDestinationType(qualType(unless(isInteger()))),
+          unless(hasParent(cxxStaticCastExpr())),
+          unless(hasParent(cStyleCastExpr())),
+          unless(isInTemplateInstantiation())),
+      this);
+}
+
+void DurationDivisionCheck::check(const MatchFinder::MatchResult &result) {
+  const auto *OpCall = result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall");
+  diag(OpCall->getOperatorLoc(),
+       "operator/ on absl::Duration objects performs integer division; "
+       "did you mean to use FDivDuration()?")
+      << FixItHint::CreateInsertion(OpCall->getBeginLoc(),
+                                    "absl::FDivDuration(")
+      << FixItHint::CreateReplacement(
+             SourceRange(OpCall->getOperatorLoc(), OpCall->getOperatorLoc()),
+             ", ")
+      << FixItHint::CreateInsertion(
+             Lexer::getLocForEndOfToken(
+                 result.SourceManager->getSpellingLoc(OpCall->getEndLoc()), 0,
+                 *result.SourceManager, result.Context->getLangOpts()),
+             ")");
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h

@@ -0,0 +1,35 @@
+//===--- DurationDivisionCheck.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_DURATIONDIVISIONCHECK_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+// Find potential incorrect uses of integer division of absl::Duration objects.
+//
+// For the user-facing documentation see: 
+// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-division.html
+
+class DurationDivisionCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+  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_DURATIONDIVISIONCHECK_H_

clang-tools-extra/docs/ReleaseNotes.rst

@@ -57,6 +57,13 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New :doc:`abseil-duration-division
+  <clang-tidy/checks/abseil-duration-division>` check.
+
+  Checks for uses of ``absl::Duration`` division that is done in a
+  floating-point context, and recommends the use of a function that
+  returns a floating-point value.
+
 - New :doc:`readability-magic-numbers
   <clang-tidy/checks/readability-magic-numbers>` check.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -4,6 +4,7 @@ Clang-Tidy Checks
 =================
 
 .. toctree::
+   abseil-duration-division
    abseil-string-find-startswith
    android-cloexec-accept
    android-cloexec-accept4

clang-tools-extra/test/clang-tidy/abseil-duration-division.cpp

@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+}  // namespace absl
+
+void TakesDouble(double);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+  absl::Duration d;
+
+  const double num_double = d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+  // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+  const float num_float = d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+  const auto SomeVal = 1.0 + d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+  if (MACRO_EQ(d/d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+  if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+  // This one generates a message, but no fix.
+  if (MACRO_DIVEQ(d, d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+ 
+  TakesDouble(d/d);
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesInt(int);
+template <class T>
+void TakesGeneric(T);
+
+void Negatives() {
+  absl::Duration d;
+  const int num_int = d/d;
+  const long num_long = d/d;
+  const short num_short = d/d;
+  const char num_char = d/d;
+  const auto num_auto = d/d;
+  const auto SomeVal = 1 + d/d;
+
+  TakesInt(d/d);
+  TakesGeneric(d/d);
+  // Explicit cast should disable the warning.
+  const double num_cast1 = static_cast<double>(d/d);
+  const double num_cast2 = (double)(d/d);
+}
+
+template <class T>
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+//This also won't trigger a warning
+void TemplateDivision() {
+  absl::Duration d;
+  DoubleDivision(d, d);
+}