Merge from 'main' to 'sycl-web' (#2)

  CONFLICT (content): Merge conflict in llvm/test/Transforms/OpenMP/hide_mem_transfer_latency.ll

Author: abhinavgaba

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

@@ -7,9 +7,9 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnnecessaryCopyInitialization.h"
-
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
+#include "../utils/LexerUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/Basic/Diagnostic.h"
@@ -21,6 +21,7 @@ namespace {
 
 using namespace ::clang::ast_matchers;
 using llvm::StringRef;
+using utils::decl_ref_expr::allDeclRefExprs;
 using utils::decl_ref_expr::isOnlyUsedAsConst;
 
 static constexpr StringRef ObjectArgId = "objectArg";
@@ -37,6 +38,19 @@ void recordFixes(const VarDecl &Var, ASTContext &Context,
   }
 }
 
+void recordRemoval(const DeclStmt &Stmt, ASTContext &Context,
+                   DiagnosticBuilder &Diagnostic) {
+  // Attempt to remove the whole line until the next non-comment token.
+  auto Tok = utils::lexer::findNextTokenSkippingComments(
+      Stmt.getEndLoc(), Context.getSourceManager(), Context.getLangOpts());
+  if (Tok) {
+    Diagnostic << FixItHint::CreateRemoval(SourceRange(
+        Stmt.getBeginLoc(), Tok->getLocation().getLocWithOffset(-1)));
+  } else {
+    Diagnostic << FixItHint::CreateRemoval(Stmt.getSourceRange());
+  }
+}
+
 AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningMethodCall) {
   // Match method call expressions where the `this` argument is only used as
   // const, this will be checked in `check()` part. This returned const
@@ -118,6 +132,11 @@ static bool isInitializingVariableImmutable(const VarDecl &InitializingVar,
   return false;
 }
 
+bool isVariableUnused(const VarDecl &Var, const Stmt &BlockStmt,
+                      ASTContext &Context) {
+  return allDeclRefExprs(Var, BlockStmt, Context).empty();
+}
+
 } // namespace
 
 UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
@@ -169,14 +188,13 @@ void UnnecessaryCopyInitialization::check(
   const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>(ObjectArgId);
   const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+  const auto *Stmt = Result.Nodes.getNodeAs<DeclStmt>("declStmt");
 
   TraversalKindScope RAII(*Result.Context, TK_AsIs);
 
   // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
   // since we cannot place them correctly.
-  bool IssueFix =
-      Result.Nodes.getNodeAs<DeclStmt>("declStmt")->isSingleDecl() &&
-      !NewVar->getLocation().isMacroID();
+  bool IssueFix = Stmt->isSingleDecl() && !NewVar->getLocation().isMacroID();
 
   // A constructor that looks like T(const T& t, bool arg = false) counts as a
   // copy only when it is called with default arguments for the arguments after
@@ -186,47 +204,68 @@ void UnnecessaryCopyInitialization::check(
       return;
 
   if (OldVar == nullptr) {
-    handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, ObjectArg,
+    handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Stmt, IssueFix, ObjectArg,
                                *Result.Context);
   } else {
-    handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix,
+    handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Stmt, IssueFix,
                            *Result.Context);
   }
 }
 
 void UnnecessaryCopyInitialization::handleCopyFromMethodReturn(
-    const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix,
-    const VarDecl *ObjectArg, ASTContext &Context) {
+    const VarDecl &Var, const Stmt &BlockStmt, const DeclStmt &Stmt,
+    bool IssueFix, const VarDecl *ObjectArg, ASTContext &Context) {
   bool IsConstQualified = Var.getType().isConstQualified();
   if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context))
     return;
   if (ObjectArg != nullptr &&
       !isInitializingVariableImmutable(*ObjectArg, BlockStmt, Context))
     return;
-
-  auto Diagnostic =
-      diag(Var.getLocation(),
-           "the %select{|const qualified }0variable %1 is copy-constructed "
-           "from a const reference%select{ but is only used as const "
-           "reference|}0; consider making it a const reference")
-      << IsConstQualified << &Var;
-  if (IssueFix)
-    recordFixes(Var, Context, Diagnostic);
+  if (isVariableUnused(Var, BlockStmt, Context)) {
+    auto Diagnostic =
+        diag(Var.getLocation(),
+             "the %select{|const qualified }0variable %1 is copy-constructed "
+             "from a const reference but is never used; consider "
+             "removing the statement")
+        << IsConstQualified << &Var;
+    if (IssueFix)
+      recordRemoval(Stmt, Context, Diagnostic);
+  } else {
+    auto Diagnostic =
+        diag(Var.getLocation(),
+             "the %select{|const qualified }0variable %1 is copy-constructed "
+             "from a const reference%select{ but is only used as const "
+             "reference|}0; consider making it a const reference")
+        << IsConstQualified << &Var;
+    if (IssueFix)
+      recordFixes(Var, Context, Diagnostic);
+  }
 }
 
 void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
     const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt,
-    bool IssueFix, ASTContext &Context) {
+    const DeclStmt &Stmt, bool IssueFix, ASTContext &Context) {
   if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) ||
       !isInitializingVariableImmutable(OldVar, BlockStmt, Context))
     return;
 
-  auto Diagnostic = diag(NewVar.getLocation(),
-                         "local copy %0 of the variable %1 is never modified; "
-                         "consider avoiding the copy")
-                    << &NewVar << &OldVar;
-  if (IssueFix)
-    recordFixes(NewVar, Context, Diagnostic);
+  if (isVariableUnused(NewVar, BlockStmt, Context)) {
+    auto Diagnostic = diag(NewVar.getLocation(),
+                           "local copy %0 of the variable %1 is never modified "
+                           "and never used; "
+                           "consider removing the statement")
+                      << &NewVar << &OldVar;
+    if (IssueFix)
+      recordRemoval(Stmt, Context, Diagnostic);
+  } else {
+    auto Diagnostic =
+        diag(NewVar.getLocation(),
+             "local copy %0 of the variable %1 is never modified; "
+             "consider avoiding the copy")
+        << &NewVar << &OldVar;
+    if (IssueFix)
+      recordFixes(NewVar, Context, Diagnostic);
+  }
 }
 
 void UnnecessaryCopyInitialization::storeOptions(

clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h

@@ -35,11 +35,12 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
 
 private:
   void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
-                                  bool IssueFix, const VarDecl *ObjectArg,
+                                  const DeclStmt &Stmt, bool IssueFix,
+                                  const VarDecl *ObjectArg,
                                   ASTContext &Context);
   void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
-                              const Stmt &BlockStmt, bool IssueFix,
-                              ASTContext &Context);
+                              const Stmt &BlockStmt, const DeclStmt &Stmt,
+                              bool IssueFix, ASTContext &Context);
   const std::vector<std::string> AllowedTypes;
 };
 

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-allowed-types.cpp

@@ -34,6 +34,7 @@ struct smart_ref {
 
 struct OtherType {
   ~OtherType();
+  void constMethod() const;
 };
 
 template <typename T> struct SomeComplexTemplate {
@@ -89,6 +90,7 @@ void positiveOtherType() {
   const auto O = getOtherType();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
   // CHECK-FIXES: const auto& O = getOtherType();
+  O.constMethod();
 }
 
 void negativeNotTooComplexRef() {