https://github.com/legrosbuffle updated https://github.com/llvm/llvm-project/pull/73921
>From 851460af6526f175bc34b105a0f5f130a2f1c6b1 Mon Sep 17 00:00:00 2001 From: Clement Courbet <cour...@google.com> Date: Thu, 30 Nov 2023 11:08:51 +0100 Subject: [PATCH 1/5] [clang-tidy] performance-unnecessary-copy-init Refactor diagnostic emission and add a hook so that derived checks can observe for which variables a warning has been emitted. --- .../UnnecessaryCopyInitialization.cpp | 73 +++++++++---------- .../UnnecessaryCopyInitialization.h | 7 ++ 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 990e20400fbfc..a9ef3faf8c343 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -15,6 +15,7 @@ #include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" #include <optional> +#include <utility> namespace clang::tidy::performance { namespace { @@ -302,6 +303,19 @@ void UnnecessaryCopyInitialization::check( } } +void UnnecessaryCopyInitialization::makeDiagnostic( + DiagnosticBuilder Diagnostic, const VarDecl &Var, const Stmt &BlockStmt, + const DeclStmt &Stmt, ASTContext &Context, bool IssueFix) { + const bool IsVarUnused = isVariableUnused(Var, BlockStmt, Context); + Diagnostic << &Var << IsVarUnused; + if (!IssueFix) + return; + if (IsVarUnused) + recordRemoval(Stmt, Context, Diagnostic); + else + recordFixes(Var, Context, Diagnostic); +} + void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( const VarDecl &Var, const Stmt &BlockStmt, const DeclStmt &Stmt, bool IssueFix, const VarDecl *ObjectArg, ASTContext &Context) { @@ -312,52 +326,33 @@ void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( !isInitializingVariableImmutable(*ObjectArg, BlockStmt, Context, ExcludedContainerTypes)) return; - 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); - } + + auto Diagnostic = + diag(Var.getLocation(), + "the %select{|const qualified }0variable %1 is copy-constructed " + "from a const reference%select{" + "%select{ but is only used as const reference|}0" + "| but is never used}2; consider " + "%select{making it a const reference|removing the statement}2") + << IsConstQualified; + makeDiagnostic(std::move(Diagnostic), Var, BlockStmt, Stmt, Context, + IssueFix); } void UnnecessaryCopyInitialization::handleCopyFromLocalVar( - const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, + const VarDecl &Var, const VarDecl &OldVar, const Stmt &BlockStmt, const DeclStmt &Stmt, bool IssueFix, ASTContext &Context) { - if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) || + if (!isOnlyUsedAsConst(Var, BlockStmt, Context) || !isInitializingVariableImmutable(OldVar, BlockStmt, Context, ExcludedContainerTypes)) return; - - 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); - } + auto Diagnostic = diag(Var.getLocation(), + "local copy %1 of the variable %0 is never modified" + "%select{| and never used}2; consider " + "%select{avoiding the copy|removing the statement}2") + << &OldVar; + makeDiagnostic(std::move(Diagnostic), Var, BlockStmt, Stmt, Context, + IssueFix); } void UnnecessaryCopyInitialization::storeOptions( diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h index ea009ba9979de..0eadca8c0137c 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -32,6 +32,12 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck { void check(const ast_matchers::MatchFinder::MatchResult &Result) override; void storeOptions(ClangTidyOptions::OptionMap &Opts) override; +protected: + // This is virtual so that derived classes can implement additional behavior. + virtual void makeDiagnostic(DiagnosticBuilder Diagnostic, const VarDecl &Var, + const Stmt &BlockStmt, const DeclStmt &Stmt, + ASTContext &Context, bool IssueFix); + private: void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, const DeclStmt &Stmt, bool IssueFix, @@ -40,6 +46,7 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck { void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, const DeclStmt &Stmt, bool IssueFix, ASTContext &Context); + const std::vector<StringRef> AllowedTypes; const std::vector<StringRef> ExcludedContainerTypes; }; >From a37b76a3ad04074eadf4ccb781c6f5e14bcea7eb Mon Sep 17 00:00:00 2001 From: Clement Courbet <cour...@google.com> Date: Fri, 1 Dec 2023 10:41:48 +0100 Subject: [PATCH 2/5] fixup! [clang-tidy] performance-unnecessary-copy-init --- .../UnnecessaryCopyInitialization.cpp | 113 ++++++++++-------- .../UnnecessaryCopyInitialization.h | 35 ++++-- 2 files changed, 85 insertions(+), 63 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index a9ef3faf8c343..16dff8cdd8821 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -262,21 +262,27 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { this); } +UnnecessaryCopyInitialization::CheckContext::CheckContext( + const ast_matchers::MatchFinder::MatchResult &Result) + : Var(*Result.Nodes.getNodeAs<VarDecl>("newVarDecl")), + BlockStmt(*Result.Nodes.getNodeAs<Stmt>("blockStmt")), + VarDeclStmt(*Result.Nodes.getNodeAs<DeclStmt>("declStmt")), + ASTCtx(*Result.Context), + // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros + // since we cannot place them correctly. + IssueFix(VarDeclStmt.isSingleDecl() && !Var.getLocation().isMacroID()), + IsVarUnused(isVariableUnused(Var, BlockStmt, ASTCtx)), + IsVarOnlyUsedAsConst(isOnlyUsedAsConst(Var, BlockStmt, ASTCtx)) {} + void UnnecessaryCopyInitialization::check( const MatchFinder::MatchResult &Result) { - const auto *NewVar = Result.Nodes.getNodeAs<VarDecl>("newVarDecl"); + const CheckContext Context(Result); const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>(OldVarDeclId); 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 = 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 // the first. @@ -290,69 +296,72 @@ void UnnecessaryCopyInitialization::check( // instantiations where the types differ and rely on implicit conversion would // no longer compile if we switched to a reference. if (differentReplacedTemplateParams( - NewVar->getType(), constructorArgumentType(OldVar, Result.Nodes), + Context.Var.getType(), constructorArgumentType(OldVar, Result.Nodes), *Result.Context)) return; if (OldVar == nullptr) { - handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Stmt, IssueFix, ObjectArg, - *Result.Context); + // `auto NewVar = functionCall();` + handleCopyFromMethodReturn(Context, ObjectArg); } else { - handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Stmt, IssueFix, - *Result.Context); + // `auto NewVar = OldVar;` + handleCopyFromLocalVar(Context, *OldVar); } } -void UnnecessaryCopyInitialization::makeDiagnostic( - DiagnosticBuilder Diagnostic, const VarDecl &Var, const Stmt &BlockStmt, - const DeclStmt &Stmt, ASTContext &Context, bool IssueFix) { - const bool IsVarUnused = isVariableUnused(Var, BlockStmt, Context); - Diagnostic << &Var << IsVarUnused; - if (!IssueFix) - return; - if (IsVarUnused) - recordRemoval(Stmt, Context, Diagnostic); - else - recordFixes(Var, Context, Diagnostic); -} - void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( - 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)) + const CheckContext &Ctx, const VarDecl *ObjectArg) { + bool IsConstQualified = Ctx.Var.getType().isConstQualified(); + if (!IsConstQualified && !Ctx.IsVarOnlyUsedAsConst) return; if (ObjectArg != nullptr && - !isInitializingVariableImmutable(*ObjectArg, BlockStmt, Context, + !isInitializingVariableImmutable(*ObjectArg, Ctx.BlockStmt, Ctx.ASTCtx, ExcludedContainerTypes)) return; - - auto Diagnostic = - diag(Var.getLocation(), - "the %select{|const qualified }0variable %1 is copy-constructed " - "from a const reference%select{" - "%select{ but is only used as const reference|}0" - "| but is never used}2; consider " - "%select{making it a const reference|removing the statement}2") - << IsConstQualified; - makeDiagnostic(std::move(Diagnostic), Var, BlockStmt, Stmt, Context, - IssueFix); + diagnoseCopyFromMethodReturn(Ctx, ObjectArg); } void UnnecessaryCopyInitialization::handleCopyFromLocalVar( - const VarDecl &Var, const VarDecl &OldVar, const Stmt &BlockStmt, - const DeclStmt &Stmt, bool IssueFix, ASTContext &Context) { - if (!isOnlyUsedAsConst(Var, BlockStmt, Context) || - !isInitializingVariableImmutable(OldVar, BlockStmt, Context, + const CheckContext &Ctx, const VarDecl &OldVar) { + if (!Ctx.IsVarOnlyUsedAsConst || + !isInitializingVariableImmutable(OldVar, Ctx.BlockStmt, Ctx.ASTCtx, ExcludedContainerTypes)) return; - auto Diagnostic = diag(Var.getLocation(), - "local copy %1 of the variable %0 is never modified" - "%select{| and never used}2; consider " - "%select{avoiding the copy|removing the statement}2") - << &OldVar; - makeDiagnostic(std::move(Diagnostic), Var, BlockStmt, Stmt, Context, - IssueFix); + diagnoseCopyFromLocalVar(Ctx, OldVar); +} + +void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn( + const CheckContext &Ctx, const VarDecl *ObjectArg) { + auto Diagnostic = + diag(Ctx.Var.getLocation(), + "the %select{|const qualified }0variable %1 is " + "copy-constructed " + "from a const reference%select{%select{ but is only used as const " + "reference|}0| but is never used}2; consider " + "%select{making it a const reference|removing the statement}2") + << Ctx.Var.getType().isConstQualified() << &Ctx.Var << Ctx.IsVarUnused; + maybeIssueFixes(Ctx, Diagnostic); +} +void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar( + const CheckContext &Ctx, const VarDecl &OldVar) { + auto Diagnostic = + diag(Ctx.Var.getLocation(), + "local copy %1 of the variable %0 is never modified%select{" + "| and never used}2; consider %select{avoiding the copy|removing " + "the statement}2") + << &OldVar << &Ctx.Var << Ctx.IsVarUnused; + maybeIssueFixes(Ctx, Diagnostic); +} + +void UnnecessaryCopyInitialization::maybeIssueFixes( + const CheckContext &Ctx, DiagnosticBuilder &Diagnostic) { + if (Ctx.IssueFix) { + if (Ctx.IsVarUnused) { + recordRemoval(Ctx.VarDeclStmt, Ctx.ASTCtx, Diagnostic); + } else { + recordFixes(Ctx.Var, Ctx.ASTCtx, Diagnostic); + } + } } void UnnecessaryCopyInitialization::storeOptions( diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h index 0eadca8c0137c..da48a3c4e2b2f 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -33,19 +33,32 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck { void storeOptions(ClangTidyOptions::OptionMap &Opts) override; protected: - // This is virtual so that derived classes can implement additional behavior. - virtual void makeDiagnostic(DiagnosticBuilder Diagnostic, const VarDecl &Var, - const Stmt &BlockStmt, const DeclStmt &Stmt, - ASTContext &Context, bool IssueFix); + // A helper to manipulate the state common to + // `CopyFromMethodReturn` and `CopyFromLocalVar`. + struct CheckContext { + CheckContext(const ast_matchers::MatchFinder::MatchResult &Result); + const VarDecl &Var; + const Stmt &BlockStmt; + const DeclStmt &VarDeclStmt; + clang::ASTContext &ASTCtx; + const bool IssueFix; + const bool IsVarUnused; + const bool IsVarOnlyUsedAsConst; + }; + + // Create diagnostics. These are virtual so that derived classes can change + // behaviour. + virtual void diagnoseCopyFromMethodReturn(const CheckContext &Ctx, + const VarDecl *ObjectArg); + virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx, + const VarDecl &OldVar); private: - void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, - const DeclStmt &Stmt, bool IssueFix, - const VarDecl *ObjectArg, - ASTContext &Context); - void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, - const Stmt &BlockStmt, const DeclStmt &Stmt, - bool IssueFix, ASTContext &Context); + void handleCopyFromMethodReturn(const CheckContext &Ctx, + const VarDecl *ObjectArg); + void handleCopyFromLocalVar(const CheckContext &Ctx, const VarDecl &OldVar); + + void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic); const std::vector<StringRef> AllowedTypes; const std::vector<StringRef> ExcludedContainerTypes; >From 1b76bb69dbac1fc2f11325cecf64990ef8732f66 Mon Sep 17 00:00:00 2001 From: Clement Courbet <cour...@google.com> Date: Wed, 6 Dec 2023 13:24:10 +0100 Subject: [PATCH 3/5] fixup! fixup! [clang-tidy] performance-unnecessary-copy-init --- .../clang-tidy/performance/UnnecessaryCopyInitialization.cpp | 5 +++-- .../clang-tidy/performance/UnnecessaryCopyInitialization.h | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 16dff8cdd8821..12f89aae9b7df 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -318,7 +318,7 @@ void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( !isInitializingVariableImmutable(*ObjectArg, Ctx.BlockStmt, Ctx.ASTCtx, ExcludedContainerTypes)) return; - diagnoseCopyFromMethodReturn(Ctx, ObjectArg); + diagnoseCopyFromMethodReturn(Ctx); } void UnnecessaryCopyInitialization::handleCopyFromLocalVar( @@ -331,7 +331,7 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar( } void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn( - const CheckContext &Ctx, const VarDecl *ObjectArg) { + const CheckContext &Ctx) { auto Diagnostic = diag(Ctx.Var.getLocation(), "the %select{|const qualified }0variable %1 is " @@ -342,6 +342,7 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn( << Ctx.Var.getType().isConstQualified() << &Ctx.Var << Ctx.IsVarUnused; maybeIssueFixes(Ctx, Diagnostic); } + void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar( const CheckContext &Ctx, const VarDecl &OldVar) { auto Diagnostic = diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h index da48a3c4e2b2f..b8c795c7e89d5 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -48,8 +48,7 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck { // Create diagnostics. These are virtual so that derived classes can change // behaviour. - virtual void diagnoseCopyFromMethodReturn(const CheckContext &Ctx, - const VarDecl *ObjectArg); + virtual void diagnoseCopyFromMethodReturn(const CheckContext &Ctx); virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx, const VarDecl &OldVar); >From fd825df8ec0994333d8bda94d45f01d7fd291aec Mon Sep 17 00:00:00 2001 From: Clement Courbet <cour...@google.com> Date: Wed, 6 Dec 2023 18:18:31 +0100 Subject: [PATCH 4/5] fixup! fixup! fixup! [clang-tidy] performance-unnecessary-copy-init --- .../UnnecessaryCopyInitialization.cpp | 27 ++++++++++--------- .../UnnecessaryCopyInitialization.h | 1 - 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 12f89aae9b7df..b44d35f5bb65b 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -262,21 +262,22 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { this); } -UnnecessaryCopyInitialization::CheckContext::CheckContext( - const ast_matchers::MatchFinder::MatchResult &Result) - : Var(*Result.Nodes.getNodeAs<VarDecl>("newVarDecl")), - BlockStmt(*Result.Nodes.getNodeAs<Stmt>("blockStmt")), - VarDeclStmt(*Result.Nodes.getNodeAs<DeclStmt>("declStmt")), - ASTCtx(*Result.Context), - // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros - // since we cannot place them correctly. - IssueFix(VarDeclStmt.isSingleDecl() && !Var.getLocation().isMacroID()), - IsVarUnused(isVariableUnused(Var, BlockStmt, ASTCtx)), - IsVarOnlyUsedAsConst(isOnlyUsedAsConst(Var, BlockStmt, ASTCtx)) {} - void UnnecessaryCopyInitialization::check( const MatchFinder::MatchResult &Result) { - const CheckContext Context(Result); + const auto &NewVar = *Result.Nodes.getNodeAs<VarDecl>("newVarDecl"); + const auto &BlockStmt = *Result.Nodes.getNodeAs<Stmt>("blockStmt"); + const auto &VarDeclStmt = *Result.Nodes.getNodeAs<DeclStmt>("declStmt"); + const CheckContext Context{ + NewVar, BlockStmt, VarDeclStmt, *Result.Context, + // Do not propose fixes if the DeclStmt has multiple VarDecls or in + // macros since we cannot place them correctly. + /*IssueFix*/ VarDeclStmt.isSingleDecl() && + !NewVar.getLocation().isMacroID(), + /*IsVarUnused*/ isVariableUnused(NewVar, BlockStmt, *Result.Context), + /*IsVarOnlyUsedAsConst*/ + isOnlyUsedAsConst(NewVar, BlockStmt, *Result.Context) + + }; const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>(OldVarDeclId); const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>(ObjectArgId); const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall"); diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h index b8c795c7e89d5..ab0f1ecf61063 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -36,7 +36,6 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck { // A helper to manipulate the state common to // `CopyFromMethodReturn` and `CopyFromLocalVar`. struct CheckContext { - CheckContext(const ast_matchers::MatchFinder::MatchResult &Result); const VarDecl &Var; const Stmt &BlockStmt; const DeclStmt &VarDeclStmt; >From f7351e7d356b17fc15c06206ee9d705fba978f6c Mon Sep 17 00:00:00 2001 From: Clement Courbet <cour...@google.com> Date: Wed, 6 Dec 2023 19:08:23 +0100 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! [clang-tidy] performance-unnecessary-copy-init --- .../UnnecessaryCopyInitialization.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index b44d35f5bb65b..dfe12c5b6007d 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -267,17 +267,16 @@ void UnnecessaryCopyInitialization::check( const auto &NewVar = *Result.Nodes.getNodeAs<VarDecl>("newVarDecl"); const auto &BlockStmt = *Result.Nodes.getNodeAs<Stmt>("blockStmt"); const auto &VarDeclStmt = *Result.Nodes.getNodeAs<DeclStmt>("declStmt"); + // Do not propose fixes if the DeclStmt has multiple VarDecls or in + // macros since we cannot place them correctly. + const bool IssueFix = + VarDeclStmt.isSingleDecl() && !NewVar.getLocation().isMacroID(); + const bool IsVarUnused = isVariableUnused(NewVar, BlockStmt, *Result.Context); + const bool IsVarOnlyUsedAsConst = + isOnlyUsedAsConst(NewVar, BlockStmt, *Result.Context); const CheckContext Context{ - NewVar, BlockStmt, VarDeclStmt, *Result.Context, - // Do not propose fixes if the DeclStmt has multiple VarDecls or in - // macros since we cannot place them correctly. - /*IssueFix*/ VarDeclStmt.isSingleDecl() && - !NewVar.getLocation().isMacroID(), - /*IsVarUnused*/ isVariableUnused(NewVar, BlockStmt, *Result.Context), - /*IsVarOnlyUsedAsConst*/ - isOnlyUsedAsConst(NewVar, BlockStmt, *Result.Context) - - }; + NewVar, BlockStmt, VarDeclStmt, *Result.Context, + IssueFix, IsVarUnused, IsVarOnlyUsedAsConst}; const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>(OldVarDeclId); const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>(ObjectArgId); const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall"); @@ -358,11 +357,10 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar( void UnnecessaryCopyInitialization::maybeIssueFixes( const CheckContext &Ctx, DiagnosticBuilder &Diagnostic) { if (Ctx.IssueFix) { - if (Ctx.IsVarUnused) { + if (Ctx.IsVarUnused) recordRemoval(Ctx.VarDeclStmt, Ctx.ASTCtx, Diagnostic); - } else { + else recordFixes(Ctx.Var, Ctx.ASTCtx, Diagnostic); - } } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits