Author: jonastoth Date: Mon Dec 3 10:59:27 2018 New Revision: 348165 URL: http://llvm.org/viewvc/llvm-project?rev=348165&view=rev Log: Revert "[clang-tidy] Add the abseil-duration-comparison check"
This commit broke buildbots and needs adjustments. Removed: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.h clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-comparison.rst clang-tools-extra/trunk/test/clang-tidy/abseil-duration-comparison.cpp Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryFloatCheck.cpp clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp clang-tools-extra/trunk/docs/ReleaseNotes.rst clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst 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=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Mon Dec 3 10:59:27 2018 @@ -10,7 +10,6 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" -#include "DurationComparisonCheck.h" #include "DurationDivisionCheck.h" #include "DurationFactoryFloatCheck.h" #include "DurationFactoryScaleCheck.h" @@ -28,8 +27,6 @@ namespace abseil { class AbseilModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.registerCheck<DurationComparisonCheck>( - "abseil-duration-comparison"); CheckFactories.registerCheck<DurationDivisionCheck>( "abseil-duration-division"); CheckFactories.registerCheck<DurationFactoryFloatCheck>( 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=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Mon Dec 3 10:59:27 2018 @@ -2,11 +2,9 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyAbseilModule AbseilTidyModule.cpp - DurationComparisonCheck.cpp DurationDivisionCheck.cpp DurationFactoryFloatCheck.cpp DurationFactoryScaleCheck.cpp - DurationRewriter.cpp FasterStrsplitDelimiterCheck.cpp NoInternalDependenciesCheck.cpp NoNamespaceCheck.cpp Removed: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp (removed) @@ -1,164 +0,0 @@ -//===--- DurationComparisonCheck.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 "DurationComparisonCheck.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 { - -/// Given the name of an inverse Duration function (e.g., `ToDoubleSeconds`), -/// return its `DurationScale`, or `None` if a match is not found. -static llvm::Optional<DurationScale> getScaleForInverse(llvm::StringRef Name) { - static const llvm::DenseMap<llvm::StringRef, DurationScale> ScaleMap( - {{"ToDoubleHours", DurationScale::Hours}, - {"ToInt64Hours", DurationScale::Hours}, - {"ToDoubleMinutes", DurationScale::Minutes}, - {"ToInt64Minutes", DurationScale::Minutes}, - {"ToDoubleSeconds", DurationScale::Seconds}, - {"ToInt64Seconds", DurationScale::Seconds}, - {"ToDoubleMilliseconds", DurationScale::Milliseconds}, - {"ToInt64Milliseconds", DurationScale::Milliseconds}, - {"ToDoubleMicroseconds", DurationScale::Microseconds}, - {"ToInt64Microseconds", DurationScale::Microseconds}, - {"ToDoubleNanoseconds", DurationScale::Nanoseconds}, - {"ToInt64Nanoseconds", DurationScale::Nanoseconds}}); - - auto ScaleIter = ScaleMap.find(std::string(Name)); - if (ScaleIter == ScaleMap.end()) - return llvm::None; - - return ScaleIter->second; -} - -/// Given a `Scale` return the inverse functions for it. -static const std::pair<llvm::StringRef, llvm::StringRef> & -getInverseForScale(DurationScale Scale) { - static const std::unordered_map<DurationScale, - std::pair<llvm::StringRef, llvm::StringRef>> - InverseMap( - {{DurationScale::Hours, - std::make_pair("::absl::ToDoubleHours", "::absl::ToInt64Hours")}, - {DurationScale::Minutes, std::make_pair("::absl::ToDoubleMinutes", - "::absl::ToInt64Minutes")}, - {DurationScale::Seconds, std::make_pair("::absl::ToDoubleSeconds", - "::absl::ToInt64Seconds")}, - {DurationScale::Milliseconds, - std::make_pair("::absl::ToDoubleMilliseconds", - "::absl::ToInt64Milliseconds")}, - {DurationScale::Microseconds, - std::make_pair("::absl::ToDoubleMicroseconds", - "::absl::ToInt64Microseconds")}, - {DurationScale::Nanoseconds, - std::make_pair("::absl::ToDoubleNanoseconds", - "::absl::ToInt64Nanoseconds")}}); - - // We know our map contains all the Scale values, so we can skip the - // nonexistence check. - auto InverseIter = InverseMap.find(Scale); - assert(InverseIter != InverseMap.end() && "Unexpected scale found"); - return InverseIter->second; -} - -/// If `Node` is a call to the inverse of `Scale`, return that inverse's -/// argument, otherwise None. -static llvm::Optional<std::string> -maybeRewriteInverseDurationCall(const MatchFinder::MatchResult &Result, - DurationScale Scale, const Expr &Node) { - const std::pair<std::string, std::string> &InverseFunctions = - getInverseForScale(Scale); - if (const Expr *MaybeCallArg = selectFirst<const Expr>( - "e", match(callExpr(callee(functionDecl( - hasAnyName(InverseFunctions.first.c_str(), - InverseFunctions.second.c_str()))), - hasArgument(0, expr().bind("e"))), - Node, *Result.Context))) { - return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str(); - } - - return llvm::None; -} - -/// Assuming `Node` has type `double` or `int` representing a time interval of -/// `Scale`, return the expression to make it a suitable `Duration`. -static llvm::Optional<std::string> rewriteExprFromNumberToDuration( - const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale, - const Expr *Node) { - const Expr &RootNode = *Node->IgnoreParenImpCasts(); - - if (RootNode.getBeginLoc().isMacroID()) - return llvm::None; - - // First check to see if we can undo a complimentary function call. - if (llvm::Optional<std::string> MaybeRewrite = - maybeRewriteInverseDurationCall(Result, Scale, RootNode)) - return *MaybeRewrite; - - if (IsLiteralZero(Result, RootNode)) - return std::string("absl::ZeroDuration()"); - - return (llvm::Twine(getFactoryForScale(Scale)) + "(" + - simplifyDurationFactoryArg(Result, RootNode) + ")") - .str(); -} - -void DurationComparisonCheck::registerMatchers(MatchFinder *Finder) { - auto Matcher = - binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="), - hasOperatorName("=="), hasOperatorName("<="), - hasOperatorName("<")), - hasEitherOperand(ignoringImpCasts(callExpr( - callee(functionDecl(DurationConversionFunction()) - .bind("function_decl")))))) - .bind("binop"); - - Finder->addMatcher(Matcher, this); -} - -void DurationComparisonCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Binop = Result.Nodes.getNodeAs<BinaryOperator>("binop"); - - // Don't try to replace things inside of macro definitions. - if (Binop->getExprLoc().isMacroID()) - return; - - llvm::Optional<DurationScale> Scale = getScaleForInverse( - Result.Nodes.getNodeAs<FunctionDecl>("function_decl")->getName()); - if (!Scale) - return; - - // In most cases, we'll only need to rewrite one of the sides, but we also - // 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. - llvm::Optional<std::string> LhsReplacement = - rewriteExprFromNumberToDuration(Result, *Scale, Binop->getLHS()); - llvm::Optional<std::string> RhsReplacement = - rewriteExprFromNumberToDuration(Result, *Scale, Binop->getRHS()); - - if (!(LhsReplacement && RhsReplacement)) - return; - - diag(Binop->getBeginLoc(), "perform comparison in the duration domain") - << FixItHint::CreateReplacement(Binop->getSourceRange(), - (llvm::Twine(*LhsReplacement) + " " + - Binop->getOpcodeStr() + " " + - *RhsReplacement) - .str()); -} - -} // namespace abseil -} // namespace tidy -} // namespace clang Removed: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.h?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.h (removed) @@ -1,36 +0,0 @@ -//===--- DurationComparisonCheck.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_DURATIONCOMPARISONCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONCOMPARISONCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace abseil { - -/// Prefer comparison in the absl::Duration domain instead of the numeric -/// domain. -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-comparison.html -class DurationComparisonCheck : public ClangTidyCheck { -public: - DurationComparisonCheck(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_DURATIONCOMPARISONCHECK_H Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryFloatCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryFloatCheck.cpp?rev=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryFloatCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryFloatCheck.cpp Mon Dec 3 10:59:27 2018 @@ -8,7 +8,6 @@ //===----------------------------------------------------------------------===// #include "DurationFactoryFloatCheck.h" -#include "DurationRewriter.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Tooling/FixIt.h" @@ -19,6 +18,19 @@ namespace clang { namespace tidy { namespace abseil { +// Returns an integer if the fractional part of a `FloatingLiteral` is `0`. +static llvm::Optional<llvm::APSInt> +truncateIfIntegral(const FloatingLiteral &FloatLiteral) { + double Value = FloatLiteral.getValueAsApproximateDouble(); + if (std::fmod(Value, 1) == 0) { + if (Value >= static_cast<double>(1u << 31)) + return llvm::None; + + return llvm::APSInt::get(static_cast<int64_t>(Value)); + } + return llvm::None; +} + // Returns `true` if `Range` is inside a macro definition. static bool InsideMacroDefinition(const MatchFinder::MatchResult &Result, SourceRange Range) { @@ -30,14 +42,21 @@ static bool InsideMacroDefinition(const void DurationFactoryFloatCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - callExpr(callee(functionDecl(DurationFactoryFunction())), - hasArgument(0, anyOf(cxxStaticCastExpr(hasDestinationType( - realFloatingPointType())), - cStyleCastExpr(hasDestinationType( - realFloatingPointType())), - cxxFunctionalCastExpr(hasDestinationType( - realFloatingPointType())), - floatLiteral()))) + callExpr( + callee(functionDecl(hasAnyName( + "absl::Nanoseconds", "absl::Microseconds", "absl::Milliseconds", + "absl::Seconds", "absl::Minutes", "absl::Hours"))), + hasArgument(0, + anyOf(cxxStaticCastExpr( + hasDestinationType(realFloatingPointType()), + hasSourceExpression(expr().bind("cast_arg"))), + cStyleCastExpr( + hasDestinationType(realFloatingPointType()), + hasSourceExpression(expr().bind("cast_arg"))), + cxxFunctionalCastExpr( + hasDestinationType(realFloatingPointType()), + hasSourceExpression(expr().bind("cast_arg"))), + floatLiteral().bind("float_literal")))) .bind("call"), this); } @@ -54,16 +73,31 @@ void DurationFactoryFloatCheck::check(co if (Arg->getBeginLoc().isMacroID()) return; - llvm::Optional<std::string> SimpleArg = stripFloatCast(Result, *Arg); - if (!SimpleArg) - SimpleArg = stripFloatLiteralFraction(Result, *Arg); - - if (SimpleArg) { + // Check for casts to `float` or `double`. + if (const auto *MaybeCastArg = Result.Nodes.getNodeAs<Expr>("cast_arg")) { diag(MatchedCall->getBeginLoc(), (llvm::Twine("use the integer version of absl::") + MatchedCall->getDirectCallee()->getName()) .str()) - << FixItHint::CreateReplacement(Arg->getSourceRange(), *SimpleArg); + << FixItHint::CreateReplacement( + Arg->getSourceRange(), + tooling::fixit::getText(*MaybeCastArg, *Result.Context)); + return; + } + + // Check for floats without fractional components. + if (const auto *LitFloat = + Result.Nodes.getNodeAs<FloatingLiteral>("float_literal")) { + // Attempt to simplify a `Duration` factory call with a literal argument. + if (llvm::Optional<llvm::APSInt> IntValue = truncateIfIntegral(*LitFloat)) { + diag(MatchedCall->getBeginLoc(), + (llvm::Twine("use the integer version of absl::") + + MatchedCall->getDirectCallee()->getName()) + .str()) + << FixItHint::CreateReplacement(LitFloat->getSourceRange(), + IntValue->toString(/*radix=*/10)); + return; + } } } Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp?rev=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationFactoryScaleCheck.cpp Mon Dec 3 10:59:27 2018 @@ -8,7 +8,6 @@ //===----------------------------------------------------------------------===// #include "DurationFactoryScaleCheck.h" -#include "DurationRewriter.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Tooling/FixIt.h" @@ -19,6 +18,20 @@ namespace clang { namespace tidy { namespace abseil { +namespace { + +// Potential scales of our inputs. +enum class DurationScale { + Hours, + Minutes, + Seconds, + Milliseconds, + Microseconds, + Nanoseconds, +}; + +} // namespace + // Given the name of a duration factory function, return the appropriate // `DurationScale` for that factory. If no factory can be found for // `FactoryName`, return `None`. @@ -116,14 +129,39 @@ static llvm::Optional<DurationScale> Get return llvm::None; } +// Given a `Scale`, return the appropriate factory function call for +// constructing a `Duration` for that scale. +static llvm::StringRef GetFactoryForScale(DurationScale Scale) { + switch (Scale) { + case DurationScale::Hours: + return "absl::Hours"; + case DurationScale::Minutes: + return "absl::Minutes"; + case DurationScale::Seconds: + return "absl::Seconds"; + case DurationScale::Milliseconds: + return "absl::Milliseconds"; + case DurationScale::Microseconds: + return "absl::Microseconds"; + case DurationScale::Nanoseconds: + return "absl::Nanoseconds"; + } + llvm_unreachable("unknown scaling factor"); +} + void DurationFactoryScaleCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( callExpr( - callee(functionDecl(DurationFactoryFunction()).bind("call_decl")), + callee(functionDecl( + hasAnyName("::absl::Nanoseconds", "::absl::Microseconds", + "::absl::Milliseconds", "::absl::Seconds", + "::absl::Minutes", "::absl::Hours")) + .bind("call_decl")), hasArgument( 0, ignoringImpCasts(anyOf( - integerLiteral(equals(0)), floatLiteral(equals(0.0)), + integerLiteral(equals(0)).bind("zero"), + floatLiteral(equals(0.0)).bind("zero"), binaryOperator(hasOperatorName("*"), hasEitherOperand(ignoringImpCasts( anyOf(integerLiteral(), floatLiteral())))) @@ -147,7 +185,7 @@ void DurationFactoryScaleCheck::check(co return; // We first handle the cases of literal zero (both float and integer). - if (IsLiteralZero(Result, *Arg)) { + if (Result.Nodes.getNodeAs<Stmt>("zero")) { diag(Call->getBeginLoc(), "use ZeroDuration() for zero-length time intervals") << FixItHint::CreateReplacement(Call->getSourceRange(), @@ -206,7 +244,7 @@ void DurationFactoryScaleCheck::check(co diag(Call->getBeginLoc(), "internal duration scaling can be removed") << FixItHint::CreateReplacement( Call->getSourceRange(), - (llvm::Twine(getFactoryForScale(*NewScale)) + "(" + + (llvm::Twine(GetFactoryForScale(*NewScale)) + "(" + tooling::fixit::getText(*Remainder, *Result.Context) + ")") .str()); } @@ -219,7 +257,7 @@ void DurationFactoryScaleCheck::check(co diag(Call->getBeginLoc(), "internal duration scaling can be removed") << FixItHint::CreateReplacement( Call->getSourceRange(), - (llvm::Twine(getFactoryForScale(*NewScale)) + "(" + + (llvm::Twine(GetFactoryForScale(*NewScale)) + "(" + tooling::fixit::getText(*Remainder, *Result.Context) + ")") .str()); } Removed: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (removed) @@ -1,109 +0,0 @@ -//===--- DurationRewriter.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 "DurationRewriter.h" -#include "clang/Tooling/FixIt.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace abseil { - -/// Returns an integer if the fractional part of a `FloatingLiteral` is `0`. -static llvm::Optional<llvm::APSInt> -truncateIfIntegral(const FloatingLiteral &FloatLiteral) { - double Value = FloatLiteral.getValueAsApproximateDouble(); - if (std::fmod(Value, 1) == 0) { - if (Value >= static_cast<double>(1u << 31)) - return llvm::None; - - return llvm::APSInt::get(static_cast<int64_t>(Value)); - } - return llvm::None; -} - -/// Returns the factory function name for a given `Scale`. -llvm::StringRef getFactoryForScale(DurationScale Scale) { - switch (Scale) { - case DurationScale::Hours: - return "absl::Hours"; - case DurationScale::Minutes: - return "absl::Minutes"; - case DurationScale::Seconds: - return "absl::Seconds"; - case DurationScale::Milliseconds: - return "absl::Milliseconds"; - case DurationScale::Microseconds: - return "absl::Microseconds"; - case DurationScale::Nanoseconds: - return "absl::Nanoseconds"; - } - llvm_unreachable("unknown scaling factor"); -} - -/// Returns `true` if `Node` is a value which evaluates to a literal `0`. -bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) { - return selectFirst<const clang::Expr>( - "val", - match(expr(ignoringImpCasts(anyOf(integerLiteral(equals(0)), - floatLiteral(equals(0.0))))) - .bind("val"), - Node, *Result.Context)) != nullptr; -} - -llvm::Optional<std::string> -stripFloatCast(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &Node) { - if (const Expr *MaybeCastArg = selectFirst<const Expr>( - "cast_arg", - match(expr(anyOf(cxxStaticCastExpr( - hasDestinationType(realFloatingPointType()), - hasSourceExpression(expr().bind("cast_arg"))), - cStyleCastExpr( - hasDestinationType(realFloatingPointType()), - hasSourceExpression(expr().bind("cast_arg"))), - cxxFunctionalCastExpr( - hasDestinationType(realFloatingPointType()), - hasSourceExpression(expr().bind("cast_arg"))))), - Node, *Result.Context))) - return tooling::fixit::getText(*MaybeCastArg, *Result.Context).str(); - - return llvm::None; -} - -llvm::Optional<std::string> -stripFloatLiteralFraction(const MatchFinder::MatchResult &Result, - const Expr &Node) { - if (const auto *LitFloat = llvm::dyn_cast<FloatingLiteral>(&Node)) - // Attempt to simplify a `Duration` factory call with a literal argument. - if (llvm::Optional<llvm::APSInt> IntValue = truncateIfIntegral(*LitFloat)) - return IntValue->toString(/*radix=*/10); - - return llvm::None; -} - -std::string simplifyDurationFactoryArg(const MatchFinder::MatchResult &Result, - const Expr &Node) { - // Check for an explicit cast to `float` or `double`. - if (llvm::Optional<std::string> MaybeArg = stripFloatCast(Result, Node)) - return *MaybeArg; - - // Check for floats without fractional components. - if (llvm::Optional<std::string> MaybeArg = - stripFloatLiteralFraction(Result, Node)) - return *MaybeArg; - - // We couldn't simplify any further, so return the argument text. - return tooling::fixit::getText(Node, *Result.Context).str(); -} - -} // namespace abseil -} // namespace tidy -} // namespace clang Removed: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h (removed) @@ -1,86 +0,0 @@ -//===--- DurationRewriter.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_DURATIONREWRITER_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONREWRITER_H - -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include <cinttypes> - -namespace clang { -namespace tidy { -namespace abseil { - -/// Duration factory and conversion scales -enum class DurationScale : std::int8_t { - Hours, - Minutes, - Seconds, - Milliseconds, - Microseconds, - Nanoseconds, -}; - -/// Given a `Scale`, return the appropriate factory function call for -/// constructing a `Duration` for that scale. -llvm::StringRef getFactoryForScale(DurationScale Scale); - -// Determine if `Node` represents a literal floating point or integral zero. -bool IsLiteralZero(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &Node); - -/// Possibly strip a floating point cast expression. -/// -/// If `Node` represents an explicit cast to a floating point type, return -/// the textual context of the cast argument, otherwise `None`. -llvm::Optional<std::string> -stripFloatCast(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &Node); - -/// Possibly remove the fractional part of a floating point literal. -/// -/// If `Node` represents a floating point literal with a zero fractional part, -/// return the textual context of the integral part, otherwise `None`. -llvm::Optional<std::string> -stripFloatLiteralFraction(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &Node); - -/// Possibly further simplify a duration factory function's argument, without -/// changing the scale of the factory function. Return that simplification or -/// the text of the argument if no simplification is possible. -std::string -simplifyDurationFactoryArg(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &Node); - -AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>, - DurationConversionFunction) { - using namespace clang::ast_matchers; - return functionDecl( - hasAnyName("::absl::ToDoubleHours", "::absl::ToDoubleMinutes", - "::absl::ToDoubleSeconds", "::absl::ToDoubleMilliseconds", - "::absl::ToDoubleMicroseconds", "::absl::ToDoubleNanoseconds", - "::absl::ToInt64Hours", "::absl::ToInt64Minutes", - "::absl::ToInt64Seconds", "::absl::ToInt64Milliseconds", - "::absl::ToInt64Microseconds", "::absl::ToInt64Nanoseconds")); -} - -AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>, - DurationFactoryFunction) { - using namespace clang::ast_matchers; - return functionDecl(hasAnyName("::absl::Nanoseconds", "::absl::Microseconds", - "::absl::Milliseconds", "::absl::Seconds", - "::absl::Minutes", "::absl::Hours")); -} - -} // namespace abseil -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONCOMPARISONCHECK_H Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Dec 3 10:59:27 2018 @@ -67,12 +67,6 @@ The improvements are... Improvements to clang-tidy -------------------------- -- New :doc:`abseil-duration-comparison - <clang-tidy/checks/abseil-duration-comparison>` check. - - Checks for comparisons which should be done in the ``absl::Duration`` domain - instead of the float of integer domains. - - New :doc:`abseil-duration-division <clang-tidy/checks/abseil-duration-division>` check. Removed: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-comparison.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-comparison.rst?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-comparison.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-comparison.rst (removed) @@ -1,33 +0,0 @@ -.. title:: clang-tidy - abseil-duration-comparison - -abseil-duration-comparison -========================== - -Checks for comparisons which should be in the ``absl::Duration`` domain instead -of the floating point or integer domains. - -N.B.: In cases where a ``Duration`` was being converted to an integer and then -compared against a floating-point value, truncation during the ``Duration`` -conversion might yield a different result. In practice this is very rare, and -still indicates a bug which should be fixed. - -Examples: - -.. code-block:: c++ - - // Original - Comparison in the floating point domain - double x; - absl::Duration d; - if (x < absl::ToDoubleSeconds(d)) ... - - // Suggested - Compare in the absl::Duration domain instead - if (absl::Seconds(x) < d) ... - - - // Original - Comparison in the integer domain - int x; - absl::Duration d; - if (x < absl::ToInt64Microseconds(d)) ... - - // Suggested - Compare in the absl::Duration domain instead - if (absl::Microseconds(x) < d) ... 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=348165&r1=348164&r2=348165&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Dec 3 10:59:27 2018 @@ -4,7 +4,6 @@ Clang-Tidy Checks ================= .. toctree:: - abseil-duration-comparison abseil-duration-division abseil-duration-factory-float abseil-duration-factory-scale Removed: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-comparison.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-comparison.cpp?rev=348164&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-comparison.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-comparison.cpp (removed) @@ -1,195 +0,0 @@ -// RUN: %check_clang_tidy %s abseil-duration-comparison %t - -// Mimic the implementation of absl::Duration -namespace absl { - -class Duration {}; -class Time{}; - -Duration Nanoseconds(long long); -Duration Microseconds(long long); -Duration Milliseconds(long long); -Duration Seconds(long long); -Duration Minutes(long long); -Duration Hours(long long); - -#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \ - Duration NAME(float n); \ - Duration NAME(double n); \ - template <typename T> \ - Duration NAME(T n); - -GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Seconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Minutes); -GENERATE_DURATION_FACTORY_OVERLOADS(Hours); -#undef GENERATE_DURATION_FACTORY_OVERLOADS - -using int64_t = long long int; - -double ToDoubleHours(Duration d); -double ToDoubleMinutes(Duration d); -double ToDoubleSeconds(Duration d); -double ToDoubleMilliseconds(Duration d); -double ToDoubleMicroseconds(Duration d); -double ToDoubleNanoseconds(Duration d); -int64_t ToInt64Hours(Duration d); -int64_t ToInt64Minutes(Duration d); -int64_t ToInt64Seconds(Duration d); -int64_t ToInt64Milliseconds(Duration d); -int64_t ToInt64Microseconds(Duration d); -int64_t ToInt64Nanoseconds(Duration d); - -// Relational Operators -constexpr bool operator<(Duration lhs, Duration rhs); -constexpr bool operator>(Duration lhs, Duration rhs); -constexpr bool operator>=(Duration lhs, Duration rhs); -constexpr bool operator<=(Duration lhs, Duration rhs); -constexpr bool operator==(Duration lhs, Duration rhs); -constexpr bool operator!=(Duration lhs, Duration rhs); - -// Additive Operators -inline Time operator+(Time lhs, Duration rhs); -inline Time operator+(Duration lhs, Time rhs); -inline Time operator-(Time lhs, Duration rhs); -inline Duration operator-(Time lhs, Time rhs); - -} // namespace absl - -void f() { - double x; - absl::Duration d1, d2; - bool b; - absl::Time t1, t2; - - // Check against the RHS - b = x > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) > d1; - b = x >= absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) >= d1; - b = x == absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) == d1; - b = x <= absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) <= d1; - b = x < absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) < d1; - b = x == absl::ToDoubleSeconds(t1 - t2); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) == t1 - t2; - b = absl::ToDoubleSeconds(d1) > absl::ToDoubleSeconds(d2); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 > d2; - - // Check against the LHS - b = absl::ToDoubleSeconds(d1) < x; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 < absl::Seconds(x); - b = absl::ToDoubleSeconds(d1) <= x; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 <= absl::Seconds(x); - b = absl::ToDoubleSeconds(d1) == x; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 == absl::Seconds(x); - b = absl::ToDoubleSeconds(d1) >= x; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 >= absl::Seconds(x); - b = absl::ToDoubleSeconds(d1) > x; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 > absl::Seconds(x); - - // Comparison against zero - b = absl::ToDoubleSeconds(d1) < 0.0; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 < absl::ZeroDuration(); - b = absl::ToDoubleSeconds(d1) < 0; - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: d1 < absl::ZeroDuration(); - - // Scales other than Seconds - b = x > absl::ToDoubleMicroseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Microseconds(x) > d1; - b = x >= absl::ToDoubleMilliseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Milliseconds(x) >= d1; - b = x == absl::ToDoubleNanoseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Nanoseconds(x) == d1; - b = x <= absl::ToDoubleMinutes(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Minutes(x) <= d1; - b = x < absl::ToDoubleHours(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Hours(x) < d1; - - // Integer comparisons - b = x > absl::ToInt64Microseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Microseconds(x) > d1; - b = x >= absl::ToInt64Milliseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Milliseconds(x) >= d1; - b = x == absl::ToInt64Nanoseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Nanoseconds(x) == d1; - b = x == absl::ToInt64Seconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(x) == d1; - b = x <= absl::ToInt64Minutes(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Minutes(x) <= d1; - b = x < absl::ToInt64Hours(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Hours(x) < d1; - - // Other abseil-duration checks folded into this one - b = static_cast<double>(5) > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(5) > d1; - b = double(5) > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(5) > d1; - b = float(5) > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(5) > d1; - b = ((double)5) > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(5) > d1; - b = 5.0 > absl::ToDoubleSeconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Seconds(5) > d1; - - // A long expression - bool some_condition; - int very_very_very_very_long_variable_name; - absl::Duration SomeDuration; - if (some_condition && very_very_very_very_long_variable_name - < absl::ToDoubleSeconds(SomeDuration)) { - // CHECK-MESSAGES: [[@LINE-2]]:25: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: if (some_condition && absl::Seconds(very_very_very_very_long_variable_name) < SomeDuration) { - return; - } - - // A complex expression - int y; - b = (y + 5) * 10 > absl::ToDoubleMilliseconds(d1); - // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the duration domain [abseil-duration-comparison] - // CHECK-FIXES: absl::Milliseconds((y + 5) * 10) > d1; - - // These should not match - b = 6 < 4; - -#define TODOUBLE(x) absl::ToDoubleSeconds(x) - b = 5.0 > TODOUBLE(d1); -#undef TODOUBLE -#define THIRTY 30.0 - b = THIRTY > absl::ToDoubleSeconds(d1); -#undef THIRTY -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits