Author: hwright Date: Mon Feb 4 11:28:20 2019 New Revision: 353079 URL: http://llvm.org/viewvc/llvm-project?rev=353079&view=rev Log: [clang-tidy] Add the abseil-duration-unnecessary-conversion check
Differential Revision: https://reviews.llvm.org/D57353 Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt clang-tools-extra/trunk/docs/ReleaseNotes.rst clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=353079&r1=353078&r2=353079&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Mon Feb 4 11:28:20 2019 @@ -16,6 +16,7 @@ #include "DurationFactoryFloatCheck.h" #include "DurationFactoryScaleCheck.h" #include "DurationSubtractionCheck.h" +#include "DurationUnnecessaryConversionCheck.h" #include "FasterStrsplitDelimiterCheck.h" #include "NoInternalDependenciesCheck.h" #include "NoNamespaceCheck.h" @@ -45,6 +46,8 @@ public: "abseil-duration-factory-scale"); CheckFactories.registerCheck<DurationSubtractionCheck>( "abseil-duration-subtraction"); + CheckFactories.registerCheck<DurationUnnecessaryConversionCheck>( + "abseil-duration-unnecessary-conversion"); CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>( "abseil-faster-strsplit-delimiter"); CheckFactories.registerCheck<NoInternalDependenciesCheck>( Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=353079&r1=353078&r2=353079&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Mon Feb 4 11:28:20 2019 @@ -10,6 +10,7 @@ add_clang_library(clangTidyAbseilModule DurationFactoryScaleCheck.cpp DurationRewriter.cpp DurationSubtractionCheck.cpp + DurationUnnecessaryConversionCheck.cpp FasterStrsplitDelimiterCheck.cpp NoInternalDependenciesCheck.cpp NoNamespaceCheck.cpp Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp?rev=353079&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp (added) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp Mon Feb 4 11:28:20 2019 @@ -0,0 +1,58 @@ +//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy +//-----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DurationUnnecessaryConversionCheck.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 DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) { + for (const auto &Scale : {"Hours", "Minutes", "Seconds", "Milliseconds", + "Microseconds", "Nanoseconds"}) { + std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str(); + std::string FloatConversion = + (llvm::Twine("::absl::ToDouble") + Scale).str(); + std::string IntegerConversion = + (llvm::Twine("::absl::ToInt64") + Scale).str(); + + Finder->addMatcher( + callExpr( + callee(functionDecl(hasName(DurationFactory))), + hasArgument(0, callExpr(callee(functionDecl(hasAnyName( + FloatConversion, IntegerConversion))), + hasArgument(0, expr().bind("arg"))))) + .bind("call"), + this); + } +} + +void DurationUnnecessaryConversionCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *OuterCall = Result.Nodes.getNodeAs<Expr>("call"); + const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg"); + + if (!isNotInMacro(Result, OuterCall)) + return; + + diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions") + << FixItHint::CreateReplacement( + OuterCall->getSourceRange(), + tooling::fixit::getText(*Arg, *Result.Context)); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h?rev=353079&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h (added) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h Mon Feb 4 11:28:20 2019 @@ -0,0 +1,35 @@ +//===--- DurationUnnecessaryConversionCheck.h - clang-tidy ------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// Finds and fixes cases where ``absl::Duration`` values are being converted +/// to numeric types and back again. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-unnecessary-conversion.html +class DurationUnnecessaryConversionCheck : public ClangTidyCheck { +public: + DurationUnnecessaryConversionCheck(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_TIMEDOUBLECONVERSIONCHECK_H Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=353079&r1=353078&r2=353079&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Feb 4 11:28:20 2019 @@ -79,6 +79,12 @@ Improvements to clang-tidy Checks for casts of ``absl::Duration`` conversion functions, and recommends the right conversion function instead. +- New :doc:`abseil-duration-unnecessary-conversion + <clang-tidy/checks/abseil-duration-unnecessary-conversion>` check. + + Finds and fixes cases where ``absl::Duration`` values are being converted to + numeric types and back again. + - New :doc:`google-readability-avoid-underscore-in-googletest-name <clang-tidy/checks/google-readability-avoid-underscore-in-googletest-name>` check. Added: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst?rev=353079&view=auto ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst (added) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst Mon Feb 4 11:28:20 2019 @@ -0,0 +1,31 @@ +.. title:: clang-tidy - abseil-duration-unnecessary-conversion + +abseil-duration-unnecessary-conversion +====================================== + +Finds and fixes cases where ``absl::Duration`` values are being converted to +numeric types and back again. + +Examples: + +.. code-block:: c++ + + // Original - Conversion to double and back again + absl::Duration d1; + absl::Duration d2 = absl::Seconds(absl::ToDoubleSeconds(d1)); + + // Suggestion - Remove unnecessary conversions + absl::Duration d2 = d1; + + + // Original - Conversion to integer and back again + absl::Duration d1; + absl::Duration d2 = absl::Hours(absl::ToInt64Hours(d1)); + + // Suggestion - Remove unnecessary conversions + absl::Duration d2 = d1; + +Note: Converting to an integer and back to an ``absl::Duration`` might be a +truncating operation if the value is not aligned to the scale of conversion. +In the rare case where this is the intended result, callers should use +``absl::Trunc`` to truncate explicitly. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=353079&r1=353078&r2=353079&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Feb 4 11:28:20 2019 @@ -11,6 +11,7 @@ Clang-Tidy Checks abseil-duration-factory-float abseil-duration-factory-scale abseil-duration-subtraction + abseil-duration-unnecessary-conversion abseil-faster-strsplit-delimiter abseil-no-internal-dependencies abseil-no-namespace Modified: clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h?rev=353079&r1=353078&r2=353079&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h (original) +++ clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h Mon Feb 4 11:28:20 2019 @@ -14,6 +14,8 @@ public: Duration &operator/=(float r); Duration &operator/=(double r); template <typename T> Duration &operator/=(T r); + + Duration &operator+(Duration d); }; template <typename T> Duration operator*(Duration lhs, T rhs); Added: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp?rev=353079&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp Mon Feb 4 11:28:20 2019 @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s abseil-duration-unnecessary-conversion %t -- -- -I%S/Inputs + +#include "absl/time/time.h" + +void f() { + absl::Duration d1, d2; + + // Floating point + d2 = absl::Hours(absl::ToDoubleHours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(absl::ToDoubleMinutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(absl::ToDoubleSeconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + + // Integer point + d2 = absl::Hours(absl::ToInt64Hours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(absl::ToInt64Minutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(absl::ToInt64Seconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(absl::ToInt64Microseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + + // As macro argument +#define PLUS_FIVE_S(x) x + absl::Seconds(5) + d2 = PLUS_FIVE_S(absl::Seconds(absl::ToInt64Seconds(d1))); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: PLUS_FIVE_S(d1) +#undef PLUS_FIVE_S + + // Split by macro: should not change +#define TOSECONDS(x) absl::Seconds(x) + d2 = TOSECONDS(absl::ToInt64Seconds(d1)); +#undef TOSECONDS + + // Don't change something inside a macro definition +#define VALUE(x) absl::Hours(absl::ToInt64Hours(x)); + d2 = VALUE(d1); +#undef VALUE + + // These should not match + d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1)); + d2 = absl::Seconds(4); + int i = absl::ToInt64Milliseconds(d1); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits