Skip to content

Commit 5cd630d

Browse files
hokeinhokein
committedAug 17, 2018
[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
1 parent 39869cc commit 5cd630d

File tree

7 files changed

+181
-0
lines changed

7 files changed

+181
-0
lines changed
 

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

+3
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../ClangTidy.h"
1111
#include "../ClangTidyModule.h"
1212
#include "../ClangTidyModuleRegistry.h"
13+
#include "DurationDivisionCheck.h"
1314
#include "StringFindStartswithCheck.h"
1415

1516
namespace clang {
@@ -19,6 +20,8 @@ namespace abseil {
1920
class AbseilModule : public ClangTidyModule {
2021
public:
2122
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
23+
CheckFactories.registerCheck<DurationDivisionCheck>(
24+
"abseil-duration-division");
2225
CheckFactories.registerCheck<StringFindStartswithCheck>(
2326
"abseil-string-find-startswith");
2427
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
22

33
add_clang_library(clangTidyAbseilModule
44
AbseilTidyModule.cpp
5+
DurationDivisionCheck.cpp
56
StringFindStartswithCheck.cpp
67

78
LINK_LIBS

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

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
//===--- DurationDivisionCheck.cpp - clang-tidy----------------------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "DurationDivisionCheck.h"
11+
#include "clang/AST/ASTContext.h"
12+
#include "clang/ASTMatchers/ASTMatchFinder.h"
13+
14+
namespace clang {
15+
namespace tidy {
16+
namespace abseil {
17+
18+
using namespace clang::ast_matchers;
19+
20+
void DurationDivisionCheck::registerMatchers(MatchFinder *finder) {
21+
if (!getLangOpts().CPlusPlus)
22+
return;
23+
24+
const auto DurationExpr =
25+
expr(hasType(cxxRecordDecl(hasName("::absl::Duration"))));
26+
finder->addMatcher(
27+
implicitCastExpr(
28+
hasSourceExpression(ignoringParenCasts(
29+
cxxOperatorCallExpr(hasOverloadedOperatorName("/"),
30+
hasArgument(0, DurationExpr),
31+
hasArgument(1, DurationExpr))
32+
.bind("OpCall"))),
33+
hasImplicitDestinationType(qualType(unless(isInteger()))),
34+
unless(hasParent(cxxStaticCastExpr())),
35+
unless(hasParent(cStyleCastExpr())),
36+
unless(isInTemplateInstantiation())),
37+
this);
38+
}
39+
40+
void DurationDivisionCheck::check(const MatchFinder::MatchResult &result) {
41+
const auto *OpCall = result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall");
42+
diag(OpCall->getOperatorLoc(),
43+
"operator/ on absl::Duration objects performs integer division; "
44+
"did you mean to use FDivDuration()?")
45+
<< FixItHint::CreateInsertion(OpCall->getBeginLoc(),
46+
"absl::FDivDuration(")
47+
<< FixItHint::CreateReplacement(
48+
SourceRange(OpCall->getOperatorLoc(), OpCall->getOperatorLoc()),
49+
", ")
50+
<< FixItHint::CreateInsertion(
51+
Lexer::getLocForEndOfToken(
52+
result.SourceManager->getSpellingLoc(OpCall->getEndLoc()), 0,
53+
*result.SourceManager, result.Context->getLangOpts()),
54+
")");
55+
}
56+
57+
} // namespace abseil
58+
} // namespace tidy
59+
} // namespace clang

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

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- DurationDivisionCheck.h - clang-tidy--------------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_
11+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_
12+
13+
#include "../ClangTidy.h"
14+
15+
namespace clang {
16+
namespace tidy {
17+
namespace abseil {
18+
19+
// Find potential incorrect uses of integer division of absl::Duration objects.
20+
//
21+
// For the user-facing documentation see:
22+
// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-division.html
23+
24+
class DurationDivisionCheck : public ClangTidyCheck {
25+
public:
26+
using ClangTidyCheck::ClangTidyCheck;
27+
void registerMatchers(ast_matchers::MatchFinder *finder) override;
28+
void check(const ast_matchers::MatchFinder::MatchResult &result) override;
29+
};
30+
31+
} // namespace abseil
32+
} // namespace tidy
33+
} // namespace clang
34+
35+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_

‎clang-tools-extra/docs/ReleaseNotes.rst

+7
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ The improvements are...
5757
Improvements to clang-tidy
5858
--------------------------
5959

60+
- New :doc:`abseil-duration-division
61+
<clang-tidy/checks/abseil-duration-division>` check.
62+
63+
Checks for uses of ``absl::Duration`` division that is done in a
64+
floating-point context, and recommends the use of a function that
65+
returns a floating-point value.
66+
6067
- New :doc:`readability-magic-numbers
6168
<clang-tidy/checks/readability-magic-numbers>` check.
6269

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

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Clang-Tidy Checks
44
=================
55

66
.. toctree::
7+
abseil-duration-division
78
abseil-string-find-startswith
89
android-cloexec-accept
910
android-cloexec-accept4

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

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// RUN: %check_clang_tidy %s abseil-duration-division %t
2+
3+
namespace absl {
4+
5+
class Duration {};
6+
7+
int operator/(Duration lhs, Duration rhs);
8+
9+
double FDivDuration(Duration num, Duration den);
10+
11+
} // namespace absl
12+
13+
void TakesDouble(double);
14+
15+
#define MACRO_EQ(x, y) (x == y)
16+
#define MACRO_DIVEQ(x,y,z) (x/y == z)
17+
#define CHECK(x) (x)
18+
19+
void Positives() {
20+
absl::Duration d;
21+
22+
const double num_double = d/d;
23+
// CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
24+
// CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
25+
const float num_float = d/d;
26+
// CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
27+
// CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
28+
const auto SomeVal = 1.0 + d/d;
29+
// CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
30+
// CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
31+
if (MACRO_EQ(d/d, 0.0)) {}
32+
// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
33+
// CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
34+
if (CHECK(MACRO_EQ(d/d, 0.0))) {}
35+
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
36+
// CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
37+
38+
// This one generates a message, but no fix.
39+
if (MACRO_DIVEQ(d, d, 0.0)) {}
40+
// CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
41+
// CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
42+
43+
TakesDouble(d/d);
44+
// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
45+
// CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
46+
}
47+
48+
void TakesInt(int);
49+
template <class T>
50+
void TakesGeneric(T);
51+
52+
void Negatives() {
53+
absl::Duration d;
54+
const int num_int = d/d;
55+
const long num_long = d/d;
56+
const short num_short = d/d;
57+
const char num_char = d/d;
58+
const auto num_auto = d/d;
59+
const auto SomeVal = 1 + d/d;
60+
61+
TakesInt(d/d);
62+
TakesGeneric(d/d);
63+
// Explicit cast should disable the warning.
64+
const double num_cast1 = static_cast<double>(d/d);
65+
const double num_cast2 = (double)(d/d);
66+
}
67+
68+
template <class T>
69+
double DoubleDivision(T t1, T t2) {return t1/t2;}
70+
71+
//This also won't trigger a warning
72+
void TemplateDivision() {
73+
absl::Duration d;
74+
DoubleDivision(d, d);
75+
}

0 commit comments

Comments
 (0)
Please sign in to comment.