Merge remote-tracking branch 'origin/master' into intel

Author: vladimirlaz

clang-tools-extra/CMakeLists.txt

@@ -9,7 +9,7 @@ add_subdirectory(modularize)
 add_subdirectory(clang-tidy)
 add_subdirectory(clang-tidy-vs)
 
-add_subdirectory(change-namespace)
+add_subdirectory(clang-change-namespace)
 add_subdirectory(clang-doc)
 add_subdirectory(clang-query)
 add_subdirectory(clang-move)

clang-tools-extra/change-namespace/CMakeLists.txt renamed to clang-tools-extra/clang-change-namespace/CMakeLists.txt

@@ -1,4 +1,4 @@
-//===-- ClangIncludeFixer.cpp - Standalone change namespace ---------------===//
+//===-- ClangChangeNamespace.cpp - Standalone change namespace ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang-tools-extra/change-namespace/ChangeNamespace.cpp renamed to clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp

@@ -1,4 +1,4 @@
-//===---- Generator.cpp - Generator Registry ---------------------*- C++-*-===//
+//===-- Generators.cpp - Generator Registry ----------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang-tools-extra/change-namespace/ChangeNamespace.h renamed to clang-tools-extra/clang-change-namespace/ChangeNamespace.h

@@ -1,4 +1,4 @@
-//===-- Serializer.cpp - ClangDoc Serializer --------------------*- C++ -*-===//
+//===-- Serialize.cpp - ClangDoc Serializer ---------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang-tools-extra/change-namespace/tool/CMakeLists.txt renamed to clang-tools-extra/clang-change-namespace/tool/CMakeLists.txt

@@ -1,4 +1,4 @@
-//===--  ClangDocYAML.cpp - ClangDoc YAML -----------------------*- C++ -*-===//
+//===-- YAMLGenerator.cpp - ClangDoc YAML -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang-tools-extra/change-namespace/tool/ClangChangeNamespace.cpp renamed to clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp

@@ -1,4 +1,4 @@
-//===-- UsedHelperDeclFinder.cpp - AST-based call graph for helper decls --===//
+//===-- HelperDeclRefGraph.cpp - AST-based call graph for helper decls ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

clang-tools-extra/clang-doc/Generators.cpp

@@ -254,7 +254,11 @@ bool ClangTidyContext::treatAsError(StringRef CheckName) const {
   return WarningAsErrorFilter->contains(CheckName);
 }
 
-StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
+std::string ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
+  std::string ClangWarningOption =
+      DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(DiagnosticID);
+  if (!ClangWarningOption.empty())
+    return "clang-diagnostic-" + ClangWarningOption;
   llvm::DenseMap<unsigned, std::string>::const_iterator I =
       CheckNamesByDiagnosticID.find(DiagnosticID);
   if (I != CheckNamesByDiagnosticID.end())
@@ -305,7 +309,7 @@ static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
           Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
       // Allow disabling all the checks with "*".
       if (ChecksStr != "*") {
-        StringRef CheckName = Context.getCheckName(DiagID);
+        std::string CheckName = Context.getCheckName(DiagID);
         // Allow specifying a few check names, delimited with comma.
         SmallVector<StringRef, 1> Checks;
         ChecksStr.split(Checks, ',', -1, false);
@@ -402,13 +406,7 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
            "A diagnostic note can only be appended to a message.");
   } else {
     finalizeLastError();
-    StringRef WarningOption =
-        Context.DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(
-            Info.getID());
-    std::string CheckName = !WarningOption.empty()
-                                ? ("clang-diagnostic-" + WarningOption).str()
-                                : Context.getCheckName(Info.getID()).str();
-
+    std::string CheckName = Context.getCheckName(Info.getID());
     if (CheckName.empty()) {
       // This is a compiler diagnostic without a warning option. Assign check
       // name based on its level.

clang-tools-extra/clang-doc/Serialize.cpp

@@ -138,7 +138,7 @@ class ClangTidyContext {
 
   /// \brief Returns the name of the clang-tidy check which produced this
   /// diagnostic ID.
-  StringRef getCheckName(unsigned DiagnosticID) const;
+  std::string getCheckName(unsigned DiagnosticID) const;
 
   /// \brief Returns \c true if the check is enabled for the \c CurrentFile.
   ///

clang-tools-extra/clang-doc/YAMLGenerator.cpp

@@ -23,6 +23,8 @@
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "TimeComparisonCheck.h"
+#include "TimeSubtractionCheck.h"
 #include "UpgradeDurationConversionsCheck.h"
 
 namespace clang {
@@ -59,6 +61,10 @@ class AbseilModule : public ClangTidyModule {
         "abseil-str-cat-append");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
         "abseil-string-find-startswith");
+    CheckFactories.registerCheck<TimeComparisonCheck>(
+        "abseil-time-comparison");
+    CheckFactories.registerCheck<TimeSubtractionCheck>(
+        "abseil-time-subtraction");
     CheckFactories.registerCheck<UpgradeDurationConversionsCheck>(
         "abseil-upgrade-duration-conversions");
   }

clang-tools-extra/clang-move/HelperDeclRefGraph.cpp

@@ -17,6 +17,8 @@ add_clang_library(clangTidyAbseilModule
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  TimeComparisonCheck.cpp
+  TimeSubtractionCheck.cpp
   UpgradeDurationConversionsCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -19,14 +19,10 @@ namespace tidy {
 namespace abseil {
 
 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");
+  auto Matcher = expr(comparisonOperatorWithCallee(functionDecl(
+                          functionDecl(DurationConversionFunction())
+                              .bind("function_decl"))))
+                     .bind("binop");
 
   Finder->addMatcher(Matcher, this);
 }
@@ -43,8 +39,7 @@ void DurationComparisonCheck::check(const MatchFinder::MatchResult &Result) {
   // want to handle the case of rewriting both sides. This is much simpler if
   // we unconditionally try and rewrite both, and let the rewriter determine
   // if nothing needs to be done.
-  if (!isNotInMacro(Result, Binop->getLHS()) ||
-      !isNotInMacro(Result, Binop->getRHS()))
+  if (isInMacro(Result, Binop->getLHS()) || isInMacro(Result, Binop->getRHS()))
     return;
   std::string LhsReplacement =
       rewriteExprFromNumberToDuration(Result, *Scale, Binop->getLHS());

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

@@ -37,7 +37,7 @@ void DurationConversionCastCheck::check(
   const auto *MatchedCast =
       Result.Nodes.getNodeAs<ExplicitCastExpr>("cast_expr");
 
-  if (!isNotInMacro(Result, MatchedCast))
+  if (isInMacro(Result, MatchedCast))
     return;
 
   const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");

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

@@ -84,6 +84,22 @@ rewriteInverseDurationCall(const MatchFinder::MatchResult &Result,
   return llvm::None;
 }
 
+/// If `Node` is a call to the inverse of `Scale`, return that inverse's
+/// argument, otherwise None.
+static llvm::Optional<std::string>
+rewriteInverseTimeCall(const MatchFinder::MatchResult &Result,
+                       DurationScale Scale, const Expr &Node) {
+  llvm::StringRef InverseFunction = getTimeInverseForScale(Scale);
+  if (const auto *MaybeCallArg = selectFirst<const Expr>(
+          "e", match(callExpr(callee(functionDecl(hasName(InverseFunction))),
+                              hasArgument(0, expr().bind("e"))),
+                     Node, *Result.Context))) {
+    return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }
+
+  return llvm::None;
+}
+
 /// Returns the factory function name for a given `Scale`.
 llvm::StringRef getDurationFactoryForScale(DurationScale Scale) {
   switch (Scale) {
@@ -103,6 +119,24 @@ llvm::StringRef getDurationFactoryForScale(DurationScale Scale) {
   llvm_unreachable("unknown scaling factor");
 }
 
+llvm::StringRef getTimeFactoryForScale(DurationScale Scale) {
+  switch (Scale) {
+  case DurationScale::Hours:
+    return "absl::FromUnixHours";
+  case DurationScale::Minutes:
+    return "absl::FromUnixMinutes";
+  case DurationScale::Seconds:
+    return "absl::FromUnixSeconds";
+  case DurationScale::Milliseconds:
+    return "absl::FromUnixMillis";
+  case DurationScale::Microseconds:
+    return "absl::FromUnixMicros";
+  case DurationScale::Nanoseconds:
+    return "absl::FromUnixNanos";
+  }
+  llvm_unreachable("unknown scaling factor");
+}
+
 /// Returns the Time factory function name for a given `Scale`.
 llvm::StringRef getTimeInverseForScale(DurationScale scale) {
   switch (scale) {
@@ -250,9 +284,27 @@ std::string rewriteExprFromNumberToDuration(
       .str();
 }
 
-bool isNotInMacro(const MatchFinder::MatchResult &Result, const Expr *E) {
+std::string rewriteExprFromNumberToTime(
+    const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
+    const Expr *Node) {
+  const Expr &RootNode = *Node->IgnoreParenImpCasts();
+
+  // First check to see if we can undo a complimentary function call.
+  if (llvm::Optional<std::string> MaybeRewrite =
+          rewriteInverseTimeCall(Result, Scale, RootNode))
+    return *MaybeRewrite;
+
+  if (IsLiteralZero(Result, RootNode))
+    return std::string("absl::UnixEpoch()");
+
+  return (llvm::Twine(getTimeFactoryForScale(Scale)) + "(" +
+          tooling::fixit::getText(RootNode, *Result.Context) + ")")
+      .str();
+}
+
+bool isInMacro(const MatchFinder::MatchResult &Result, const Expr *E) {
   if (!E->getBeginLoc().isMacroID())
-    return true;
+    return false;
 
   SourceLocation Loc = E->getBeginLoc();
   // We want to get closer towards the initial macro typed into the source only
@@ -264,7 +316,7 @@ bool isNotInMacro(const MatchFinder::MatchResult &Result, const Expr *E) {
     // because Clang comment says it "should not generally be used by clients."
     Loc = Result.SourceManager->getImmediateMacroCallerLoc(Loc);
   }
-  return !Loc.isMacroID();
+  return Loc.isMacroID();
 }
 
 } // namespace abseil

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

@@ -31,6 +31,10 @@ enum class DurationScale : std::uint8_t {
 /// constructing a `Duration` for that scale.
 llvm::StringRef getDurationFactoryForScale(DurationScale Scale);
 
+/// Given a 'Scale', return the appropriate factory function call for
+/// constructing a `Time` for that scale.
+llvm::StringRef getTimeFactoryForScale(DurationScale scale);
+
 // Determine if `Node` represents a literal floating point or integral zero.
 bool IsLiteralZero(const ast_matchers::MatchFinder::MatchResult &Result,
                    const Expr &Node);
@@ -81,10 +85,16 @@ std::string rewriteExprFromNumberToDuration(
     const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
     const Expr *Node);
 
-/// Return `true` if `E` is a either: not a macro at all; or an argument to
+/// Assuming `Node` has a type `int` representing a time instant of `Scale`
+/// since The Epoch, return the expression to make it a suitable `Time`.
+std::string rewriteExprFromNumberToTime(
+    const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
+    const Expr *Node);
+
+/// Return `false` if `E` is a either: not a macro at all; or an argument to
 /// one.  In the both cases, we often want to do the transformation.
-bool isNotInMacro(const ast_matchers::MatchFinder::MatchResult &Result,
-                  const Expr *E);
+bool isInMacro(const ast_matchers::MatchFinder::MatchResult &Result,
+               const Expr *E);
 
 AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
                      DurationConversionFunction) {
@@ -114,6 +124,16 @@ AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
       "::absl::ToUnixMillis", "::absl::ToUnixMicros", "::absl::ToUnixNanos"));
 }
 
+AST_MATCHER_FUNCTION_P(ast_matchers::internal::Matcher<Stmt>,
+                       comparisonOperatorWithCallee,
+                       ast_matchers::internal::Matcher<Decl>, funcDecl) {
+  using namespace clang::ast_matchers;
+  return binaryOperator(
+      anyOf(hasOperatorName(">"), hasOperatorName(">="), hasOperatorName("=="),
+            hasOperatorName("<="), hasOperatorName("<")),
+      hasEitherOperand(ignoringImpCasts(callExpr(callee(funcDecl)))));
+}
+
 } // namespace abseil
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/abseil/DurationComparisonCheck.cpp

@@ -28,12 +28,35 @@ void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
     std::string IntegerConversion =
         (llvm::Twine("::absl::ToInt64") + Scale).str();
 
+    // Matcher which matches the current scale's factory with a `1` argument,
+    // e.g. `absl::Seconds(1)`.
+    auto factory_matcher = cxxConstructExpr(hasArgument(
+        0,
+        callExpr(callee(functionDecl(hasName(DurationFactory))),
+                 hasArgument(0, ignoringImpCasts(integerLiteral(equals(1)))))));
+
+    // Matcher which matches either inverse function and binds its argument,
+    // e.g. `absl::ToDoubleSeconds(dur)`.
+    auto inverse_function_matcher = callExpr(
+        callee(functionDecl(hasAnyName(FloatConversion, IntegerConversion))),
+        hasArgument(0, expr().bind("arg")));
+
+    // Matcher which matches a duration divided by the factory_matcher above,
+    // e.g. `dur / absl::Seconds(1)`.
+    auto division_operator_matcher = cxxOperatorCallExpr(
+        hasOverloadedOperatorName("/"), hasArgument(0, expr().bind("arg")),
+        hasArgument(1, factory_matcher));
+
+    // Matcher which matches a duration argument to `FDivDuration`,
+    // e.g. `absl::FDivDuration(dur, absl::Seconds(1))`
+    auto fdiv_matcher = callExpr(
+        callee(functionDecl(hasName("::absl::FDivDuration"))),
+        hasArgument(0, expr().bind("arg")), hasArgument(1, factory_matcher));
+
     Finder->addMatcher(
-        callExpr(
-            callee(functionDecl(hasName(DurationFactory))),
-            hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
-                                        FloatConversion, IntegerConversion))),
-                                    hasArgument(0, expr().bind("arg")))))
+        callExpr(callee(functionDecl(hasName(DurationFactory))),
+                 hasArgument(0, anyOf(inverse_function_matcher,
+                                      division_operator_matcher, fdiv_matcher)))
             .bind("call"),
         this);
   }
@@ -44,10 +67,11 @@ void DurationUnnecessaryConversionCheck::check(
   const auto *OuterCall = Result.Nodes.getNodeAs<Expr>("call");
   const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
 
-  if (!isNotInMacro(Result, OuterCall))
+  if (isInMacro(Result, OuterCall))
     return;
 
-  diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions")
+  diag(OuterCall->getBeginLoc(),
+       "remove unnecessary absl::Duration conversions")
       << FixItHint::CreateReplacement(
              OuterCall->getSourceRange(),
              tooling::fixit::getText(*Arg, *Result.Context));