Merge "merge main into amd-staging" into amd-staging

Author: nicebert

clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp

@@ -117,8 +117,10 @@ void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
             /*SkipTrailingWhitespaceAndNewLine=*/true));
     for (const auto *UsingShadow : Using->shadows()) {
       const auto *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
-      if (shouldCheckDecl(TargetDecl))
+      if (shouldCheckDecl(TargetDecl)) {
         Context.UsingTargetDecls.insert(TargetDecl);
+        UsingTargetDeclsCache.insert(TargetDecl);
+      }
     }
     if (!Context.UsingTargetDecls.empty())
       Contexts.push_back(Context);
@@ -201,13 +203,16 @@ void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
   if (!D)
     return;
+  const Decl *CanonicalDecl = D->getCanonicalDecl();
+  if (!UsingTargetDeclsCache.contains(CanonicalDecl))
+    return;
   // FIXME: Currently, we don't handle the using-decls being used in different
   // scopes (such as different namespaces, different functions). Instead of
   // giving an incorrect message, we mark all of them as used.
-  //
-  // FIXME: Use a more efficient way to find a matching context.
   for (auto &Context : Contexts) {
-    if (Context.UsingTargetDecls.contains(D->getCanonicalDecl()))
+    if (Context.IsUsed)
+      continue;
+    if (Context.UsingTargetDecls.contains(CanonicalDecl))
       Context.IsUsed = true;
   }
 }
@@ -224,6 +229,7 @@ void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
     }
   }
   Contexts.clear();
+  UsingTargetDeclsCache.clear();
 }
 
 } // namespace clang::tidy::misc

clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h

@@ -49,6 +49,7 @@ class UnusedUsingDeclsCheck : public ClangTidyCheck {
   };
 
   std::vector<UsingDeclContext> Contexts;
+  llvm::SmallPtrSet<const Decl *, 32> UsingTargetDeclsCache;
 
   StringRef RawStringHeaderFileExtensions;
   FileExtensionsSet HeaderFileExtensions;

clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp

@@ -97,6 +97,15 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
   return true;
 }
 
+static bool isReferenced(const VarDecl &LoopVar, const Stmt &Stmt,
+                         ASTContext &Context) {
+  const auto IsLoopVar = varDecl(equalsNode(&LoopVar));
+  return !match(stmt(hasDescendant(declRefExpr(to(valueDecl(anyOf(
+                    IsLoopVar, bindingDecl(forDecomposition(IsLoopVar)))))))),
+                Stmt, Context)
+              .empty();
+}
+
 bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
     const VarDecl &LoopVar, const CXXForRangeStmt &ForRange,
     ASTContext &Context) {
@@ -113,9 +122,7 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
   // compiler warning which can't be suppressed.
   // Since this case is very rare, it is safe to ignore it.
   if (!ExprMutationAnalyzer(*ForRange.getBody(), Context).isMutated(&LoopVar) &&
-      !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
-                                             Context)
-           .empty()) {
+      isReferenced(LoopVar, *ForRange.getBody(), Context)) {
     auto Diag = diag(
         LoopVar.getLocation(),
         "loop variable is copied but only used as const reference; consider "

clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ImplicitBoolConversionCheck.h"
+#include "../utils/FixItHintUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -61,31 +62,6 @@ bool isUnaryLogicalNotOperator(const Stmt *Statement) {
   return UnaryOperatorExpr && UnaryOperatorExpr->getOpcode() == UO_LNot;
 }
 
-bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) {
-  switch (OperatorKind) {
-  case OO_New:
-  case OO_Delete: // Fall-through on purpose.
-  case OO_Array_New:
-  case OO_Array_Delete:
-  case OO_ArrowStar:
-  case OO_Arrow:
-  case OO_Call:
-  case OO_Subscript:
-    return false;
-
-  default:
-    return true;
-  }
-}
-
-bool areParensNeededForStatement(const Stmt *Statement) {
-  if (const auto *OperatorCall = dyn_cast<CXXOperatorCallExpr>(Statement)) {
-    return areParensNeededForOverloadedOperator(OperatorCall->getOperator());
-  }
-
-  return isa<BinaryOperator>(Statement) || isa<UnaryOperator>(Statement);
-}
-
 void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
                               const ImplicitCastExpr *Cast, const Stmt *Parent,
                               ASTContext &Context) {
@@ -105,9 +81,10 @@ void fixGenericExprCastToBool(DiagnosticBuilder &Diag,
 
   const Expr *SubExpr = Cast->getSubExpr();
 
-  bool NeedInnerParens = areParensNeededForStatement(SubExpr);
+  bool NeedInnerParens =
+      SubExpr != nullptr && utils::fixit::areParensNeededForStatement(*SubExpr);
   bool NeedOuterParens =
-      Parent != nullptr && areParensNeededForStatement(Parent);
+      Parent != nullptr && utils::fixit::areParensNeededForStatement(*Parent);
 
   std::string StartLocInsertion;
 
@@ -364,7 +341,7 @@ void ImplicitBoolConversionCheck::handleCastToBool(const ImplicitCastExpr *Cast,
     return;
   }
 
-  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion %0 -> bool")
+  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion %0 -> 'bool'")
               << Cast->getSubExpr()->getType();
 
   StringRef EquivalentLiteral =
@@ -381,7 +358,7 @@ void ImplicitBoolConversionCheck::handleCastFromBool(
     ASTContext &Context) {
   QualType DestType =
       NextImplicitCast ? NextImplicitCast->getType() : Cast->getType();
-  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion bool -> %0")
+  auto Diag = diag(Cast->getBeginLoc(), "implicit conversion 'bool' -> %0")
               << DestType;
 
   if (const auto *BoolLiteral =

clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp

@@ -224,6 +224,40 @@ std::optional<FixItHint> addQualifierToVarDecl(const VarDecl &Var,
   return std::nullopt;
 }
 
+bool areParensNeededForStatement(const Stmt &Node) {
+  if (isa<ParenExpr>(&Node))
+    return false;
+
+  if (isa<clang::BinaryOperator>(&Node) || isa<UnaryOperator>(&Node))
+    return true;
+
+  if (isa<clang::ConditionalOperator>(&Node) ||
+      isa<BinaryConditionalOperator>(&Node))
+    return true;
+
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(&Node)) {
+    switch (Op->getOperator()) {
+    case OO_PlusPlus:
+      [[fallthrough]];
+    case OO_MinusMinus:
+      return Op->getNumArgs() != 2;
+    case OO_Call:
+      [[fallthrough]];
+    case OO_Subscript:
+      [[fallthrough]];
+    case OO_Arrow:
+      return false;
+    default:
+      return true;
+    };
+  }
+
+  if (isa<CStyleCastExpr>(&Node))
+    return true;
+
+  return false;
+}
+
 // Return true if expr needs to be put in parens when it is an argument of a
 // prefix unary operator, e.g. when it is a binary or ternary operator
 // syntactically.

clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

@@ -47,6 +47,10 @@ addQualifierToVarDecl(const VarDecl &Var, const ASTContext &Context,
 
 // \brief Format a pointer to an expression
 std::string formatDereference(const Expr &ExprNode, const ASTContext &Context);
+
+// \brief Checks whatever a expression require extra () to be always used in
+// safe way in any other expression.
+bool areParensNeededForStatement(const Stmt &Node);
 } // namespace clang::tidy::utils::fixit
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -447,6 +447,10 @@ Changes in existing checks
   <clang-tidy/checks/performance/faster-string-find>` check to properly escape
   single quotes.
 
+- Improved :doc:`performance-for-range-copy
+  <clang-tidy/checks/performance/for-range-copy>` check to handle cases where
+  the loop variable is a structured binding.
+
 - Improved :doc:`performance-noexcept-move-constructor
   <clang-tidy/checks/performance/noexcept-move-constructor>` to better handle
   conditional ``noexcept`` expressions, eliminating false-positives.
@@ -498,8 +502,8 @@ Changes in existing checks
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to take
   do-while loops into account for the `AllowIntegerConditions` and
   `AllowPointerConditions` options. It also now provides more consistent
-  suggestions when parentheses are added to the return value. It also ignores
-  false-positives for comparison containing bool bitfield.
+  suggestions when parentheses are added to the return value or expressions. 
+  It also ignores false-positives for comparison containing bool bitfield.
 
 - Improved :doc:`readability-misleading-indentation
   <clang-tidy/checks/readability/misleading-indentation>` check to ignore

clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp

@@ -47,6 +47,11 @@ struct S {
   S &operator=(const S &);
 };
 
+struct Point {
+  ~Point() {}
+  int x, y;
+};
+
 struct Convertible {
   operator S() const {
     return S();
@@ -87,6 +92,10 @@ void instantiated() {
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: the loop variable's type is {{.*}}
   // CHECK-FIXES: {{^}}  for (const S& S2 : View<Iterator<S>>()) {}
 
+  for (const auto [X, Y] : View<Iterator<Point>>()) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: the loop variable's type is
+  // CHECK-FIXES: {{^}}  for (const auto& [X, Y] : View<Iterator<Point>>()) {}
+
   for (const T T2 : View<Iterator<T>>()) {}
   // CHECK-MESSAGES: [[@LINE-1]]:16: warning: the loop variable's type is {{.*}}
   // CHECK-FIXES: {{^}}  for (const T& T2 : View<Iterator<T>>()) {}
@@ -123,11 +132,6 @@ struct Mutable {
   ~Mutable() {}
 };
 
-struct Point {
-  ~Point() {}
-  int x, y;
-};
-
 Mutable& operator<<(Mutable &Out, bool B) {
   Out.setBool(B);
   return Out;
@@ -144,6 +148,7 @@ void useByValue(Mutable M);
 void useByConstValue(const Mutable M);
 void mutate(Mutable *M);
 void mutate(Mutable &M);
+void mutate(int &);
 void onceConstOnceMutated(const Mutable &M1, Mutable &M2);
 
 void negativeVariableIsMutated() {
@@ -234,6 +239,22 @@ void positiveOnlyAccessedFieldAsConst() {
   }
 }
 
+void positiveOnlyUsedAsConstBinding() {
+  for (auto [X, Y] : View<Iterator<Point>>()) {
+    // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but
+    // CHECK-FIXES: for (const auto& [X, Y] : View<Iterator<Point>>()) {
+    use(X);
+    use(Y);
+  }
+}
+
+void negativeMutatedBinding() {
+  for (auto [X, Y] : View<Iterator<Point>>()) {
+    use(X);
+    mutate(Y);
+  }
+}
+
 void positiveOnlyUsedInCopyConstructor() {
   for (auto A : View<Iterator<Mutable>>()) {
     // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]

clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-allow-in-conditions.cpp

@@ -19,7 +19,7 @@ struct Struct {
 void regularImplicitConversionIntegerToBoolIsNotIgnored() {
   int integer = 0;
   functionTaking<bool>(integer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]
   // CHECK-FIXES: functionTaking<bool>(integer != 0);
 }
 
@@ -54,12 +54,12 @@ void implicitConversionIntegerToBoolInConditionalsIsAllowed() {
 void regularImplicitConversionPointerToBoolIsNotIgnored() {
   int* pointer = nullptr;
   functionTaking<bool>(pointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(pointer != nullptr);
 
   int Struct::* memberPointer = &Struct::member;
   functionTaking<bool>(memberPointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(memberPointer != nullptr);
 }
 

clang-tools-extra/test/clang-tidy/checkers/readability/implicit-bool-conversion-cxx98.cpp

@@ -15,31 +15,31 @@ struct Struct {
 void useOldNullMacroInReplacements() {
   int* pointer = NULL;
   functionTaking<bool>(pointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool [readability-implicit-bool-conversion]
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> 'bool' [readability-implicit-bool-conversion]
   // CHECK-FIXES: functionTaking<bool>(pointer != 0);
 
   int Struct::* memberPointer = NULL;
   functionTaking<bool>(!memberPointer);
-  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'int Struct::*' -> bool
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'int Struct::*' -> 'bool'
   // CHECK-FIXES: functionTaking<bool>(memberPointer == 0);
 }
 
 void fixFalseLiteralConvertingToNullPointer() {
   functionTaking<int*>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int *'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'int *'
   // CHECK-FIXES: functionTaking<int*>(0);
 
   int* pointer = NULL;
   if (pointer == false) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit conversion bool -> 'int *'
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit conversion 'bool' -> 'int *'
   // CHECK-FIXES: if (pointer == 0) {}
 
   functionTaking<int Struct::*>(false);
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'int Struct::*'
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'bool' -> 'int Struct::*'
   // CHECK-FIXES: functionTaking<int Struct::*>(0);
 
   int Struct::* memberPointer = NULL;
   if (memberPointer != false) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int Struct::*'
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'bool' -> 'int Struct::*'
   // CHECK-FIXES: if (memberPointer != 0) {}
 }