Skip to content

Commit 1c250bf

Browse files
author
Hyrum Wright
committed
[clang-tidy] Add the abseil-duration-unnecessary-conversion check
Differential Revision: https://reviews.llvm.org/D57353 llvm-svn: 353079
1 parent d7fa13c commit 1c250bf

File tree

9 files changed

+206
-0
lines changed

9 files changed

+206
-0
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "DurationFactoryFloatCheck.h"
1717
#include "DurationFactoryScaleCheck.h"
1818
#include "DurationSubtractionCheck.h"
19+
#include "DurationUnnecessaryConversionCheck.h"
1920
#include "FasterStrsplitDelimiterCheck.h"
2021
#include "NoInternalDependenciesCheck.h"
2122
#include "NoNamespaceCheck.h"
@@ -45,6 +46,8 @@ class AbseilModule : public ClangTidyModule {
4546
"abseil-duration-factory-scale");
4647
CheckFactories.registerCheck<DurationSubtractionCheck>(
4748
"abseil-duration-subtraction");
49+
CheckFactories.registerCheck<DurationUnnecessaryConversionCheck>(
50+
"abseil-duration-unnecessary-conversion");
4851
CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>(
4952
"abseil-faster-strsplit-delimiter");
5053
CheckFactories.registerCheck<NoInternalDependenciesCheck>(

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_clang_library(clangTidyAbseilModule
1010
DurationFactoryScaleCheck.cpp
1111
DurationRewriter.cpp
1212
DurationSubtractionCheck.cpp
13+
DurationUnnecessaryConversionCheck.cpp
1314
FasterStrsplitDelimiterCheck.cpp
1415
NoInternalDependenciesCheck.cpp
1516
NoNamespaceCheck.cpp

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy
2+
//-----------------------===//
3+
//
4+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
5+
// See https://llvm.org/LICENSE.txt for license information.
6+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "DurationUnnecessaryConversionCheck.h"
11+
#include "DurationRewriter.h"
12+
#include "clang/AST/ASTContext.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "clang/Tooling/FixIt.h"
15+
16+
using namespace clang::ast_matchers;
17+
18+
namespace clang {
19+
namespace tidy {
20+
namespace abseil {
21+
22+
void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
23+
for (const auto &Scale : {"Hours", "Minutes", "Seconds", "Milliseconds",
24+
"Microseconds", "Nanoseconds"}) {
25+
std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str();
26+
std::string FloatConversion =
27+
(llvm::Twine("::absl::ToDouble") + Scale).str();
28+
std::string IntegerConversion =
29+
(llvm::Twine("::absl::ToInt64") + Scale).str();
30+
31+
Finder->addMatcher(
32+
callExpr(
33+
callee(functionDecl(hasName(DurationFactory))),
34+
hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
35+
FloatConversion, IntegerConversion))),
36+
hasArgument(0, expr().bind("arg")))))
37+
.bind("call"),
38+
this);
39+
}
40+
}
41+
42+
void DurationUnnecessaryConversionCheck::check(
43+
const MatchFinder::MatchResult &Result) {
44+
const auto *OuterCall = Result.Nodes.getNodeAs<Expr>("call");
45+
const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
46+
47+
if (!isNotInMacro(Result, OuterCall))
48+
return;
49+
50+
diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions")
51+
<< FixItHint::CreateReplacement(
52+
OuterCall->getSourceRange(),
53+
tooling::fixit::getText(*Arg, *Result.Context));
54+
}
55+
56+
} // namespace abseil
57+
} // namespace tidy
58+
} // namespace clang

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- DurationUnnecessaryConversionCheck.h - clang-tidy ------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
11+
12+
#include "../ClangTidy.h"
13+
14+
namespace clang {
15+
namespace tidy {
16+
namespace abseil {
17+
18+
/// Finds and fixes cases where ``absl::Duration`` values are being converted
19+
/// to numeric types and back again.
20+
///
21+
/// For the user-facing documentation see:
22+
/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-unnecessary-conversion.html
23+
class DurationUnnecessaryConversionCheck : public ClangTidyCheck {
24+
public:
25+
DurationUnnecessaryConversionCheck(StringRef Name, ClangTidyContext *Context)
26+
: ClangTidyCheck(Name, Context) {}
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_TIMEDOUBLECONVERSIONCHECK_H

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ Improvements to clang-tidy
7979
Checks for casts of ``absl::Duration`` conversion functions, and recommends
8080
the right conversion function instead.
8181

82+
- New :doc:`abseil-duration-unnecessary-conversion
83+
<clang-tidy/checks/abseil-duration-unnecessary-conversion>` check.
84+
85+
Finds and fixes cases where ``absl::Duration`` values are being converted to
86+
numeric types and back again.
87+
8288
- New :doc:`google-readability-avoid-underscore-in-googletest-name
8389
<clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name>`
8490
check.

‎clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
.. title:: clang-tidy - abseil-duration-unnecessary-conversion
2+
3+
abseil-duration-unnecessary-conversion
4+
======================================
5+
6+
Finds and fixes cases where ``absl::Duration`` values are being converted to
7+
numeric types and back again.
8+
9+
Examples:
10+
11+
.. code-block:: c++
12+
13+
// Original - Conversion to double and back again
14+
absl::Duration d1;
15+
absl::Duration d2 = absl::Seconds(absl::ToDoubleSeconds(d1));
16+
17+
// Suggestion - Remove unnecessary conversions
18+
absl::Duration d2 = d1;
19+
20+
21+
// Original - Conversion to integer and back again
22+
absl::Duration d1;
23+
absl::Duration d2 = absl::Hours(absl::ToInt64Hours(d1));
24+
25+
// Suggestion - Remove unnecessary conversions
26+
absl::Duration d2 = d1;
27+
28+
Note: Converting to an integer and back to an ``absl::Duration`` might be a
29+
truncating operation if the value is not aligned to the scale of conversion.
30+
In the rare case where this is the intended result, callers should use
31+
``absl::Trunc`` to truncate explicitly.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Clang-Tidy Checks
1111
abseil-duration-factory-float
1212
abseil-duration-factory-scale
1313
abseil-duration-subtraction
14+
abseil-duration-unnecessary-conversion
1415
abseil-faster-strsplit-delimiter
1516
abseil-no-internal-dependencies
1617
abseil-no-namespace

‎clang-tools-extra/test/clang-tidy/Inputs/absl/time/time.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ class Duration {
1414
Duration &operator/=(float r);
1515
Duration &operator/=(double r);
1616
template <typename T> Duration &operator/=(T r);
17+
18+
Duration &operator+(Duration d);
1719
};
1820

1921
template <typename T> Duration operator*(Duration lhs, T rhs);

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// RUN: %check_clang_tidy %s abseil-duration-unnecessary-conversion %t -- -- -I%S/Inputs
2+
3+
#include "absl/time/time.h"
4+
5+
void f() {
6+
absl::Duration d1, d2;
7+
8+
// Floating point
9+
d2 = absl::Hours(absl::ToDoubleHours(d1));
10+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
11+
// CHECK-FIXES: d1
12+
d2 = absl::Minutes(absl::ToDoubleMinutes(d1));
13+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
14+
// CHECK-FIXES: d1
15+
d2 = absl::Seconds(absl::ToDoubleSeconds(d1));
16+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
17+
// CHECK-FIXES: d1
18+
d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1));
19+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
20+
// CHECK-FIXES: d1
21+
d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1));
22+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
23+
// CHECK-FIXES: d1
24+
d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1));
25+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
26+
// CHECK-FIXES: d1
27+
28+
// Integer point
29+
d2 = absl::Hours(absl::ToInt64Hours(d1));
30+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
31+
// CHECK-FIXES: d1
32+
d2 = absl::Minutes(absl::ToInt64Minutes(d1));
33+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
34+
// CHECK-FIXES: d1
35+
d2 = absl::Seconds(absl::ToInt64Seconds(d1));
36+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
37+
// CHECK-FIXES: d1
38+
d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1));
39+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
40+
// CHECK-FIXES: d1
41+
d2 = absl::Microseconds(absl::ToInt64Microseconds(d1));
42+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
43+
// CHECK-FIXES: d1
44+
d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1));
45+
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
46+
// CHECK-FIXES: d1
47+
48+
// As macro argument
49+
#define PLUS_FIVE_S(x) x + absl::Seconds(5)
50+
d2 = PLUS_FIVE_S(absl::Seconds(absl::ToInt64Seconds(d1)));
51+
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
52+
// CHECK-FIXES: PLUS_FIVE_S(d1)
53+
#undef PLUS_FIVE_S
54+
55+
// Split by macro: should not change
56+
#define TOSECONDS(x) absl::Seconds(x)
57+
d2 = TOSECONDS(absl::ToInt64Seconds(d1));
58+
#undef TOSECONDS
59+
60+
// Don't change something inside a macro definition
61+
#define VALUE(x) absl::Hours(absl::ToInt64Hours(x));
62+
d2 = VALUE(d1);
63+
#undef VALUE
64+
65+
// These should not match
66+
d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1));
67+
d2 = absl::Seconds(4);
68+
int i = absl::ToInt64Milliseconds(d1);
69+
}

0 commit comments

Comments
 (0)