[clang-tools-extra] r348165 - Revert "[clang-tidy] Add the abseil-duration-comparison check"
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 3 10:59:28 PST 2018
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
-}
More information about the cfe-commits
mailing list