deannagarcia updated this revision to Diff 160336.
deannagarcia marked 6 inline comments as done.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===================================================================
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,66 @@
+// 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 TakesInt(int);
+
+#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 TakesDouble(double);
+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);
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
=================
.. toctree::
+ abseil-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===================================================================
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+========================
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link <https://github.com/abseil/abseil-cpp/blob/29ff6d4860070bf8fcbd39c8805d0c32d56628a3/absl/time/time.h#L137>`_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks 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.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,7 +57,12 @@
Improvements to clang-tidy
--------------------------
-The improvements are...
+- 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.
Improvements to include-fixer
-----------------------------
Index: clang-tidy/abseil/DurationDivisionCheck.h
===================================================================
--- clang-tidy/abseil/DurationDivisionCheck.h
+++ 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_
Index: clang-tidy/abseil/DurationDivisionCheck.cpp
===================================================================
--- clang-tidy/abseil/DurationDivisionCheck.cpp
+++ clang-tidy/abseil/DurationDivisionCheck.cpp
@@ -0,0 +1,58 @@
+//===--- 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()))),
+ 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->getLocStart(),
+ "absl::FDivDuration(")
+ << FixItHint::CreateReplacement(
+ SourceRange(OpCall->getOperatorLoc(), OpCall->getOperatorLoc()),
+ ", ")
+ << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(
+ result.SourceManager->getSpellingLoc(OpCall->getLocEnd()), 0,
+ *result.SourceManager, result.Context->getLangOpts()),
+ ")");
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
+ DurationDivisionCheck.cpp
StringFindStartswithCheck.cpp
LINK_LIBS
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ 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 @@
class AbseilModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<DurationDivisionCheck>(
+ "abseil-duration-division");
CheckFactories.registerCheck<StringFindStartswithCheck>(
"abseil-string-find-startswith");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
