hwright updated this revision to Diff 181204.
hwright marked an inline comment as done.
hwright added a comment.
Run `clang-format`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56532/new/
https://reviews.llvm.org/D56532
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tidy/abseil/DurationConversionCastCheck.cpp
clang-tidy/abseil/DurationConversionCastCheck.h
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-conversion-cast.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-conversion-cast.cpp
Index: test/clang-tidy/abseil-duration-conversion-cast.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/abseil-duration-conversion-cast.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s abseil-duration-conversion-cast %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+ absl::Duration d1;
+ double x;
+ int i;
+
+ i = static_cast<int>(absl::ToDoubleHours(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Hours(d1);
+ x = static_cast<float>(absl::ToInt64Hours(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleHours(d1);
+ i = static_cast<int>(absl::ToDoubleMinutes(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Minutes(d1);
+ x = static_cast<float>(absl::ToInt64Minutes(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleMinutes(d1);
+ i = static_cast<int>(absl::ToDoubleSeconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Seconds(d1);
+ x = static_cast<float>(absl::ToInt64Seconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleSeconds(d1);
+ i = static_cast<int>(absl::ToDoubleMilliseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Milliseconds(d1);
+ x = static_cast<float>(absl::ToInt64Milliseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleMilliseconds(d1);
+ i = static_cast<int>(absl::ToDoubleMicroseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Microseconds(d1);
+ x = static_cast<float>(absl::ToInt64Microseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleMicroseconds(d1);
+ i = static_cast<int>(absl::ToDoubleNanoseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Nanoseconds(d1);
+ x = static_cast<float>(absl::ToInt64Nanoseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleNanoseconds(d1);
+
+ // Functional-style casts
+ i = int(absl::ToDoubleHours(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Hours(d1);
+ x = float(absl::ToInt64Microseconds(d1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleMicroseconds(d1);
+
+ // C-style casts
+ i = (int) absl::ToDoubleHours(d1);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToInt64Hours(d1);
+ x = (float) absl::ToInt64Microseconds(d1);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: convert duration directly to double [abseil-duration-conversion-cast]
+ // CHECK-FIXES: absl::ToDoubleMicroseconds(d1);
+
+ // Macro handling
+ // We want to transform things in macro arguments
+#define EXTERNAL(x) (x) + 5
+ i = EXTERNAL(static_cast<int>(absl::ToDoubleSeconds(d1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: convert duration directly to integer [abseil-duration-conversion-cast]
+ // CHECK-FIXES: EXTERNAL(absl::ToInt64Seconds(d1));
+#undef EXTERNAL
+
+ // We don't want to transform this which get split across macro boundaries
+#define SPLIT(x) static_cast<int>((x)) + 5
+ i = SPLIT(absl::ToDoubleSeconds(d1));
+#undef SPLIT
+
+ // We also don't want to transform things inside of a macro definition
+#define INTERNAL(x) static_cast<int>(absl::ToDoubleSeconds((x))) + 5
+ i = INTERNAL(d1);
+#undef INTERNAL
+
+ // These shouldn't be converted
+ i = static_cast<int>(absl::ToInt64Seconds(d1));
+ i = static_cast<float>(absl::ToDoubleSeconds(d1));
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-duration-comparison
+ abseil-duration-conversion-cast
abseil-duration-division
abseil-duration-factory-float
abseil-duration-factory-scale
Index: docs/clang-tidy/checks/abseil-duration-conversion-cast.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/abseil-duration-conversion-cast.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - abseil-duration-conversion-cast
+
+abseil-duration-conversion-cast
+===============================
+
+Checks for casts of ``absl::Duration`` conversion functions, and recommends
+the right conversion function instead.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - Cast from a double to an integer
+ absl::Duration d;
+ int i = static_cast<int>(absl::ToDoubleSeconds(d));
+
+ // Suggested - Use the integer conversion function directly.
+ int i = absl::ToInt64Seconds(d);
+
+
+ // Original - Cast from a double to an integer
+ absl::Duration d;
+ double x = static_cast<double>(absl::ToInt64Seconds(d));
+
+ // Suggested - Use the integer conversion function directly.
+ double x = absl::ToDoubleSeconds(d);
+
+
+Note: In the second example, the suggested fix could yield a different result,
+as the conversion to integer could truncate. In practice, this is very rare,
+and you should use ``absl::Trunc`` to perform this operation explicitly instead.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -113,6 +113,12 @@
Checks for comparisons which should be done in the ``absl::Duration`` domain
instead of the float of integer domains.
+- New :doc:`abseil-duration-conversion-cast
+ <clang-tidy/checks/abseil-duration-conversion-cast>` check.
+
+ Checks for casts of ``absl::Duration`` conversion functions, and recommends
+ the right conversion function instead.
+
- New :doc:`abseil-duration-division
<clang-tidy/checks/abseil-duration-division>` check.
Index: clang-tidy/abseil/DurationRewriter.h
===================================================================
--- clang-tidy/abseil/DurationRewriter.h
+++ clang-tidy/abseil/DurationRewriter.h
@@ -75,6 +75,11 @@
const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
const Expr *Node);
+/// Return `true` if `E` is a either: not a macro at all; or an argument to
+/// one. In the both cases, we often want to do the transformation.
+bool isNotInMacro(const ast_matchers::MatchFinder::MatchResult &Result,
+ const Expr *E);
+
AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
DurationConversionFunction) {
using namespace clang::ast_matchers;
Index: clang-tidy/abseil/DurationRewriter.cpp
===================================================================
--- clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tidy/abseil/DurationRewriter.cpp
@@ -216,6 +216,25 @@
.str();
}
+/// Return `true` if `E` is a either: not a macro at all; or an argument to
+/// one. In the latter case, we should still transform it.
+bool isNotInMacro(const MatchFinder::MatchResult &Result, const Expr *E) {
+ if (!E->getBeginLoc().isMacroID())
+ return true;
+
+ SourceLocation Loc = E->getBeginLoc();
+ // We want to get closer towards the initial macro typed into the source only
+ // if the location is being expanded as a macro argument.
+ while (Result.SourceManager->isMacroArgExpansion(Loc)) {
+ // We are calling getImmediateMacroCallerLoc, but note it is essentially
+ // equivalent to calling getImmediateSpellingLoc in this context according
+ // to Clang implementation. We are not calling getImmediateSpellingLoc
+ // because Clang comment says it "should not generally be used by clients."
+ Loc = Result.SourceManager->getImmediateMacroCallerLoc(Loc);
+ }
+ return !Loc.isMacroID();
+}
+
} // namespace abseil
} // namespace tidy
} // namespace clang
Index: clang-tidy/abseil/DurationConversionCastCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/abseil/DurationConversionCastCheck.h
@@ -0,0 +1,36 @@
+//===--- DurationConversionCastCheck.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_DURATIONCONVERSIONCASTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONCONVERSIONCASTCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Checks for casts of ``absl::Duration`` conversion functions, and recommends
+/// the right conversion function instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-conversion-cast.html
+class DurationConversionCastCheck : public ClangTidyCheck {
+public:
+ DurationConversionCastCheck(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_DURATIONCONVERSIONCASTCHECK_H
Index: clang-tidy/abseil/DurationConversionCastCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/abseil/DurationConversionCastCheck.cpp
@@ -0,0 +1,81 @@
+//===--- DurationConversionCastCheck.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 "DurationConversionCastCheck.h"
+#include "DurationRewriter.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void DurationConversionCastCheck::registerMatchers(MatchFinder *Finder) {
+ auto CallMatcher = ignoringImpCasts(callExpr(
+ callee(functionDecl(DurationConversionFunction()).bind("func_decl")),
+ hasArgument(0, expr().bind("arg"))));
+
+ Finder->addMatcher(
+ expr(anyOf(
+ cxxStaticCastExpr(hasSourceExpression(CallMatcher)).bind("cast_expr"),
+ cStyleCastExpr(hasSourceExpression(CallMatcher)).bind("cast_expr"),
+ cxxFunctionalCastExpr(hasSourceExpression(CallMatcher))
+ .bind("cast_expr"))),
+ this);
+}
+
+void DurationConversionCastCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedCast =
+ Result.Nodes.getNodeAs<ExplicitCastExpr>("cast_expr");
+
+ if (!isNotInMacro(Result, MatchedCast))
+ return;
+
+ const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
+ const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
+ llvm::StringRef ConversionFuncName = FuncDecl->getName();
+
+ llvm::Optional<DurationScale> Scale = getScaleForInverse(ConversionFuncName);
+ if (!Scale)
+ return;
+
+ // Casting a double to an integer
+ if (MatchedCast->getTypeAsWritten()->isIntegerType() &&
+ ConversionFuncName.contains("Double")) {
+ llvm::StringRef NewFuncName = getInverseForScale(*Scale).second;
+
+ diag(MatchedCast->getBeginLoc(), "convert duration directly to integer")
+ << FixItHint::CreateReplacement(
+ MatchedCast->getSourceRange(),
+ (llvm::Twine(NewFuncName.substr(2)) + "(" +
+ tooling::fixit::getText(*Arg, *Result.Context) + ")")
+ .str());
+ }
+
+ // Casting an integer to a double
+ if (MatchedCast->getTypeAsWritten()->isRealFloatingType() &&
+ ConversionFuncName.contains("Int64")) {
+ llvm::StringRef NewFuncName = getInverseForScale(*Scale).first;
+
+ diag(MatchedCast->getBeginLoc(), "convert duration directly to double")
+ << FixItHint::CreateReplacement(
+ MatchedCast->getSourceRange(),
+ (llvm::Twine(NewFuncName.substr(2)) + "(" +
+ tooling::fixit::getText(*Arg, *Result.Context) + ")")
+ .str());
+ }
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/DurationComparisonCheck.cpp
===================================================================
--- clang-tidy/abseil/DurationComparisonCheck.cpp
+++ clang-tidy/abseil/DurationComparisonCheck.cpp
@@ -19,26 +19,6 @@
namespace tidy {
namespace abseil {
-/// Return `true` if `E` is a either: not a macro at all; or an argument to
-/// one. In the latter case, we should still transform it.
-static bool IsValidMacro(const MatchFinder::MatchResult &Result,
- const Expr *E) {
- if (!E->getBeginLoc().isMacroID())
- return true;
-
- SourceLocation Loc = E->getBeginLoc();
- // We want to get closer towards the initial macro typed into the source only
- // if the location is being expanded as a macro argument.
- while (Result.SourceManager->isMacroArgExpansion(Loc)) {
- // We are calling getImmediateMacroCallerLoc, but note it is essentially
- // equivalent to calling getImmediateSpellingLoc in this context according
- // to Clang implementation. We are not calling getImmediateSpellingLoc
- // because Clang comment says it "should not generally be used by clients."
- Loc = Result.SourceManager->getImmediateMacroCallerLoc(Loc);
- }
- return !Loc.isMacroID();
-}
-
void DurationComparisonCheck::registerMatchers(MatchFinder *Finder) {
auto Matcher =
binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="),
@@ -64,8 +44,8 @@
// want to handle the case of rewriting both sides. This is much simpler if
// we unconditionally try and rewrite both, and let the rewriter determine
// if nothing needs to be done.
- if (!IsValidMacro(Result, Binop->getLHS()) ||
- !IsValidMacro(Result, Binop->getRHS()))
+ if (!isNotInMacro(Result, Binop->getLHS()) ||
+ !isNotInMacro(Result, Binop->getRHS()))
return;
std::string LhsReplacement =
rewriteExprFromNumberToDuration(Result, *Scale, Binop->getLHS());
Index: clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
DurationComparisonCheck.cpp
+ DurationConversionCastCheck.cpp
DurationDivisionCheck.cpp
DurationFactoryFloatCheck.cpp
DurationFactoryScaleCheck.cpp
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tidy/abseil/AbseilTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "DurationComparisonCheck.h"
+#include "DurationConversionCastCheck.h"
#include "DurationDivisionCheck.h"
#include "DurationFactoryFloatCheck.h"
#include "DurationFactoryScaleCheck.h"
@@ -32,6 +33,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<DurationComparisonCheck>(
"abseil-duration-comparison");
+ CheckFactories.registerCheck<DurationConversionCastCheck>(
+ "abseil-duration-conversion-cast");
CheckFactories.registerCheck<DurationDivisionCheck>(
"abseil-duration-division");
CheckFactories.registerCheck<DurationFactoryFloatCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
