Merged master:b1a7a245ec2 into amd-gfx:08dd4e64e34

Local branch amd-gfx 08dd4e6 Merged master:7b74b0d4e54 into amd-gfx:46601da50bc
Remote branch master b1a7a24 [NFC][MC] Rename alignBranches* to emitInstruction*

Change-Id: Icb9cc815b8e5e8043d602f625a8f8743d50cee3c

Author: dstutt

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

@@ -52,10 +52,22 @@ void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
         callee(functionDecl(hasName("::absl::FDivDuration"))),
         hasArgument(0, expr().bind("arg")), hasArgument(1, factory_matcher));
 
+    // Matcher which matches a duration argument being scaled,
+    // e.g. `absl::ToDoubleSeconds(dur) * 2`
+    auto scalar_matcher = ignoringImpCasts(
+        binaryOperator(hasOperatorName("*"),
+                       hasEitherOperand(expr(ignoringParenImpCasts(
+                           callExpr(callee(functionDecl(hasAnyName(
+                                        FloatConversion, IntegerConversion))),
+                                    hasArgument(0, expr().bind("arg")))
+                               .bind("inner_call")))))
+            .bind("binop"));
+
     Finder->addMatcher(
         callExpr(callee(functionDecl(hasName(DurationFactory))),
                  hasArgument(0, anyOf(inverse_function_matcher,
-                                      division_operator_matcher, fdiv_matcher)))
+                                      division_operator_matcher, fdiv_matcher,
+                                      scalar_matcher)))
             .bind("call"),
         this);
   }
@@ -64,16 +76,41 @@ void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
 void DurationUnnecessaryConversionCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *OuterCall = Result.Nodes.getNodeAs<Expr>("call");
-  const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
 
   if (isInMacro(Result, OuterCall))
     return;
 
+  FixItHint Hint;
+  if (const auto *Binop = Result.Nodes.getNodeAs<BinaryOperator>("binop")) {
+    const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
+    const auto *InnerCall = Result.Nodes.getNodeAs<Expr>("inner_call");
+    const Expr *LHS = Binop->getLHS();
+    const Expr *RHS = Binop->getRHS();
+
+    if (LHS->IgnoreParenImpCasts() == InnerCall) {
+      Hint = FixItHint::CreateReplacement(
+          OuterCall->getSourceRange(),
+          (llvm::Twine(tooling::fixit::getText(*Arg, *Result.Context)) + " * " +
+           tooling::fixit::getText(*RHS, *Result.Context))
+              .str());
+    } else {
+      assert(RHS->IgnoreParenImpCasts() == InnerCall &&
+             "Inner call should be find on the RHS");
+
+      Hint = FixItHint::CreateReplacement(
+          OuterCall->getSourceRange(),
+          (llvm::Twine(tooling::fixit::getText(*LHS, *Result.Context)) + " * " +
+           tooling::fixit::getText(*Arg, *Result.Context))
+              .str());
+    }
+  } else if (const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg")) {
+    Hint = FixItHint::CreateReplacement(
+        OuterCall->getSourceRange(),
+        tooling::fixit::getText(*Arg, *Result.Context));
+  }
   diag(OuterCall->getBeginLoc(),
        "remove unnecessary absl::Duration conversions")
-      << FixItHint::CreateReplacement(
-             OuterCall->getSourceRange(),
-             tooling::fixit::getText(*Arg, *Result.Context));
+      << Hint;
 }
 
 } // namespace abseil

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

@@ -18,6 +18,8 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
+static constexpr int UnsignedASCIIUpperBound = 127;
+
 static Matcher<TypedefDecl> hasAnyListedName(const std::string &Names) {
   const std::vector<std::string> NameList =
       utils::options::parseStringList(Names);
@@ -33,70 +35,114 @@ void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
 }
 
-void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+// Create a matcher for char -> integer cast.
+BindableMatcher<clang::Stmt> SignedCharMisuseCheck::charCastExpression(
+    bool IsSigned, const Matcher<clang::QualType> &IntegerType,
+    const std::string &CastBindName) const {
   // We can ignore typedefs which are some kind of integer types
   // (e.g. typedef char sal_Int8). In this case, we don't need to
   // worry about the misinterpretation of char values.
   const auto IntTypedef = qualType(
       hasDeclaration(typedefDecl(hasAnyListedName(CharTypdefsToIgnoreList))));
 
-  const auto SignedCharType = expr(hasType(qualType(
-      allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef)))));
-
-  const auto IntegerType = qualType(allOf(isInteger(), unless(isAnyCharacter()),
-                                          unless(booleanType())))
-                               .bind("integerType");
+  auto CharTypeExpr = expr();
+  if (IsSigned) {
+    CharTypeExpr = expr(hasType(
+        qualType(isAnyCharacter(), isSignedInteger(), unless(IntTypedef))));
+  } else {
+    CharTypeExpr = expr(hasType(qualType(
+        isAnyCharacter(), unless(isSignedInteger()), unless(IntTypedef))));
+  }
 
-  // We are interested in signed char -> integer conversion.
   const auto ImplicitCastExpr =
-      implicitCastExpr(hasSourceExpression(SignedCharType),
+      implicitCastExpr(hasSourceExpression(CharTypeExpr),
                        hasImplicitDestinationType(IntegerType))
-          .bind("castExpression");
+          .bind(CastBindName);
 
   const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
   const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
   const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
 
   // We catch any type of casts to an integer. We need to have these cast
   // expressions explicitly to catch only those casts which are direct children
-  // of an assignment/declaration.
-  const auto CastExpr = expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
-                                   StaticCastExpr, FunctionalCastExpr));
+  // of the checked expressions. (e.g. assignment, declaration).
+  return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
+                    FunctionalCastExpr));
+}
 
-  // Catch assignments with the suspicious type conversion.
-  const auto AssignmentOperatorExpr = expr(binaryOperator(
-      hasOperatorName("="), hasLHS(hasType(IntegerType)), hasRHS(CastExpr)));
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IntegerType =
+      qualType(isInteger(), unless(isAnyCharacter()), unless(booleanType()))
+          .bind("integerType");
+  const auto SignedCharCastExpr =
+      charCastExpression(true, IntegerType, "signedCastExpression");
+  const auto UnSignedCharCastExpr =
+      charCastExpression(false, IntegerType, "unsignedCastExpression");
+
+  // Catch assignments with singed char -> integer conversion.
+  const auto AssignmentOperatorExpr =
+      expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)),
+                          hasRHS(SignedCharCastExpr)));
 
   Finder->addMatcher(AssignmentOperatorExpr, this);
 
-  // Catch declarations with the suspicious type conversion.
-  const auto Declaration =
-      varDecl(isDefinition(), hasType(IntegerType), hasInitializer(CastExpr));
+  // Catch declarations with singed char -> integer conversion.
+  const auto Declaration = varDecl(isDefinition(), hasType(IntegerType),
+                                   hasInitializer(SignedCharCastExpr));
 
   Finder->addMatcher(Declaration, this);
+
+  // Catch signed char/unsigned char comparison.
+  const auto CompareOperator =
+      expr(binaryOperator(hasAnyOperatorName("==", "!="),
+                          anyOf(allOf(hasLHS(SignedCharCastExpr),
+                                      hasRHS(UnSignedCharCastExpr)),
+                                allOf(hasLHS(UnSignedCharCastExpr),
+                                      hasRHS(SignedCharCastExpr)))))
+          .bind("comparison");
+
+  Finder->addMatcher(CompareOperator, this);
 }
 
 void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CastExpression =
-      Result.Nodes.getNodeAs<ImplicitCastExpr>("castExpression");
-  const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType");
-  assert(CastExpression);
-  assert(IntegerType);
+  const auto *SignedCastExpression =
+      Result.Nodes.getNodeAs<ImplicitCastExpr>("signedCastExpression");
 
-  // Ignore the match if we know that the value is not negative.
+  // Ignore the match if we know that the signed char's value is not negative.
   // The potential misinterpretation happens for negative values only.
   Expr::EvalResult EVResult;
-  if (!CastExpression->isValueDependent() &&
-      CastExpression->getSubExpr()->EvaluateAsInt(EVResult, *Result.Context)) {
-    llvm::APSInt Value1 = EVResult.Val.getInt();
-    if (Value1.isNonNegative())
+  if (!SignedCastExpression->isValueDependent() &&
+      SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
+                                                        *Result.Context)) {
+    llvm::APSInt Value = EVResult.Val.getInt();
+    if (Value.isNonNegative())
       return;
   }
 
-  diag(CastExpression->getBeginLoc(),
-       "'signed char' to %0 conversion; "
-       "consider casting to 'unsigned char' first.")
-      << *IntegerType;
+  if (const auto *Comparison = Result.Nodes.getNodeAs<Expr>("comparison")) {
+    const auto *UnSignedCastExpression =
+        Result.Nodes.getNodeAs<ImplicitCastExpr>("unsignedCastExpression");
+
+    // We can ignore the ASCII value range also for unsigned char.
+    Expr::EvalResult EVResult;
+    if (!UnSignedCastExpression->isValueDependent() &&
+        UnSignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
+                                                            *Result.Context)) {
+      llvm::APSInt Value = EVResult.Val.getInt();
+      if (Value <= UnsignedASCIIUpperBound)
+        return;
+    }
+
+    diag(Comparison->getBeginLoc(),
+         "comparison between 'signed char' and 'unsigned char'");
+  } else if (const auto *IntegerType =
+                 Result.Nodes.getNodeAs<QualType>("integerType")) {
+    diag(SignedCastExpression->getBeginLoc(),
+         "'signed char' to %0 conversion; "
+         "consider casting to 'unsigned char' first.")
+        << *IntegerType;
+  } else
+    llvm_unreachable("Unexpected match");
 }
 
 } // namespace bugprone

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h

@@ -15,13 +15,11 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
-/// Finds ``signed char`` -> integer conversions which might indicate a programming
-/// error. The basic problem with the ``signed char``, that it might store the
-/// non-ASCII characters as negative values. The human programmer probably
-/// expects that after an integer conversion the converted value matches with the
-/// character code (a value from [0..255]), however, the actual value is in
-/// [-128..127] interval. This also applies to the plain ``char`` type on
-/// those implementations which represent ``char`` similar to ``signed char``.
+/// Finds those ``signed char`` -> integer conversions which might indicate a
+/// programming error. The basic problem with the ``signed char``, that it might
+/// store the non-ASCII characters as negative values. This behavior can cause a
+/// misunderstanding of the written code both when an explicit and when an
+/// implicit conversion happens.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-signed-char-misuse.html
@@ -34,6 +32,11 @@ class SignedCharMisuseCheck : public ClangTidyCheck {
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
+  ast_matchers::internal::BindableMatcher<clang::Stmt> charCastExpression(
+      bool IsSigned,
+      const ast_matchers::internal::Matcher<clang::QualType> &IntegerType,
+      const std::string &CastBindName) const;
+
   const std::string CharTypdefsToIgnoreList;
 };
 

clang-tools-extra/clangd/Protocol.cpp

@@ -407,7 +407,9 @@ llvm::json::Value toJSON(const WorkDoneProgressBegin &P) {
     Result["cancellable"] = true;
   if (P.percentage)
     Result["percentage"] = 0;
-  return Result;
+
+  // FIXME: workaround for older gcc/clang
+  return std::move(Result);
 }
 
 llvm::json::Value toJSON(const WorkDoneProgressReport &P) {
@@ -418,14 +420,16 @@ llvm::json::Value toJSON(const WorkDoneProgressReport &P) {
     Result["message"] = *P.message;
   if (P.percentage)
     Result["percentage"] = *P.percentage;
-  return Result;
+  // FIXME: workaround for older gcc/clang
+  return std::move(Result);
 }
 
 llvm::json::Value toJSON(const WorkDoneProgressEnd &P) {
   llvm::json::Object Result{{"kind", "end"}};
   if (P.message)
     Result["message"] = *P.message;
-  return Result;
+  // FIXME: workaround for older gcc/clang
+  return std::move(Result);
 }
 
 llvm::json::Value toJSON(const MessageType &R) {
@@ -530,6 +534,7 @@ llvm::json::Value toJSON(const Diagnostic &D) {
     Diag["source"] = D.source;
   if (D.relatedInformation)
     Diag["relatedInformation"] = *D.relatedInformation;
+  // FIXME: workaround for older gcc/clang
   return std::move(Diag);
 }
 
@@ -648,8 +653,8 @@ llvm::json::Value toJSON(const SymbolDetails &P) {
   if (P.ID.hasValue())
     Result["id"] = P.ID.getValue().str();
 
-  // Older clang cannot compile 'return Result', even though it is legal.
-  return llvm::json::Value(std::move(Result));
+  // FIXME: workaround for older gcc/clang
+  return std::move(Result);
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &O, const SymbolDetails &S) {
@@ -711,8 +716,8 @@ llvm::json::Value toJSON(const DocumentSymbol &S) {
     Result["children"] = S.children;
   if (S.deprecated)
     Result["deprecated"] = true;
-  // Older gcc cannot compile 'return Result', even though it is legal.
-  return llvm::json::Value(std::move(Result));
+  // FIXME: workaround for older gcc/clang
+  return std::move(Result);
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {

clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst

@@ -40,6 +40,17 @@ Integer examples:
   // Suggestion - Remove division and conversion
   absl::Duration d2 = d1;
 
+Unwrapping scalar operations:
+
+.. code-block:: c++
+
+  // Original - Multiplication by a scalar
+  absl::Duration d1;
+  absl::Duration d2 = absl::Seconds(absl::ToInt64Seconds(d1) * 2);
+
+  // Suggestion - Remove unnecessary conversion
+  absl::Duration d2 = d1 * 2;
+
 Note: Converting to an integer and back to an ``absl::Duration`` might be a
 truncating operation if the value is not aligned to the scale of conversion.
 In the rare case where this is the intended result, callers should use