Author: Clement Courbet Date: 2024-07-23T11:32:26+02:00 New Revision: dd23b347890512ee82741648e941df24e1d666ee
URL: https://github.com/llvm/llvm-project/commit/dd23b347890512ee82741648e941df24e1d666ee DIFF: https://github.com/llvm/llvm-project/commit/dd23b347890512ee82741648e941df24e1d666ee.diff LOG: [clang-tidy][performance-unnecessary-value-param] Make `handleMoveFix` virtual (#99867) ... so that downstream checks can override behaviour to do additional processing. Refactor the rest of the logic to `handleConstRefFix` (which is also `virtual`). This is otherwise and NFC. This is similar to https://github.com/llvm/llvm-project/pull/73921 but for `performance-unnecessary-value-param`. Added: Modified: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp index 5a4c2363bd8af..3a255c5c133f1 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -119,65 +119,72 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) { } } - const size_t Index = llvm::find(Function->parameters(), Param) - - Function->parameters().begin(); + handleConstRefFix(*Function, *Param, *Result.Context); +} + +void UnnecessaryValueParamCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void UnnecessaryValueParamCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", Inserter.getStyle()); + Options.store(Opts, "AllowedTypes", + utils::options::serializeStringList(AllowedTypes)); +} + +void UnnecessaryValueParamCheck::onEndOfTranslationUnit() { + MutationAnalyzerCache.clear(); +} + +void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function, + const ParmVarDecl &Param, + ASTContext &Context) { + const size_t Index = + llvm::find(Function.parameters(), &Param) - Function.parameters().begin(); + const bool IsConstQualified = + Param.getType().getCanonicalType().isConstQualified(); auto Diag = - diag(Param->getLocation(), + diag(Param.getLocation(), "the %select{|const qualified }0parameter %1 is copied for each " "invocation%select{ but only used as a const reference|}0; consider " "making it a %select{const |}0reference") - << IsConstQualified << paramNameOrIndex(Param->getName(), Index); + << IsConstQualified << paramNameOrIndex(Param.getName(), Index); // Do not propose fixes when: // 1. the ParmVarDecl is in a macro, since we cannot place them correctly // 2. the function is virtual as it might break overrides // 3. the function is referenced outside of a call expression within the // compilation unit as the signature change could introduce build errors. // 4. the function is an explicit template/ specialization. - const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function); - if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) || - isReferencedOutsideOfCallExpr(*Function, *Result.Context) || - Function->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) + const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function); + if (Param.getBeginLoc().isMacroID() || (Method && Method->isVirtual()) || + isReferencedOutsideOfCallExpr(Function, Context) || + Function.getTemplateSpecializationKind() == TSK_ExplicitSpecialization) return; - for (const auto *FunctionDecl = Function; FunctionDecl != nullptr; + for (const auto *FunctionDecl = &Function; FunctionDecl != nullptr; FunctionDecl = FunctionDecl->getPreviousDecl()) { const auto &CurrentParam = *FunctionDecl->getParamDecl(Index); - Diag << utils::fixit::changeVarDeclToReference(CurrentParam, - *Result.Context); + Diag << utils::fixit::changeVarDeclToReference(CurrentParam, Context); // The parameter of each declaration needs to be checked individually as to // whether it is const or not as constness can diff er between definition and // declaration. if (!CurrentParam.getType().getCanonicalType().isConstQualified()) { if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl( - CurrentParam, *Result.Context, DeclSpec::TQ::TQ_const)) + CurrentParam, Context, DeclSpec::TQ::TQ_const)) Diag << *Fix; } } } -void UnnecessaryValueParamCheck::registerPPCallbacks( - const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - Inserter.registerPreprocessor(PP); -} - -void UnnecessaryValueParamCheck::storeOptions( - ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", Inserter.getStyle()); - Options.store(Opts, "AllowedTypes", - utils::options::serializeStringList(AllowedTypes)); -} - -void UnnecessaryValueParamCheck::onEndOfTranslationUnit() { - MutationAnalyzerCache.clear(); -} - -void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var, +void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Param, const DeclRefExpr &CopyArgument, - const ASTContext &Context) { + ASTContext &Context) { auto Diag = diag(CopyArgument.getBeginLoc(), "parameter %0 is passed by value and only copied once; " "consider moving it to avoid unnecessary copies") - << &Var; + << &Param; // Do not propose fixes in macros since we cannot place them correctly. if (CopyArgument.getBeginLoc().isMacroID()) return; diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h index 7250bffd20b2f..8bfd814d16357 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h @@ -33,10 +33,16 @@ class UnnecessaryValueParamCheck : public ClangTidyCheck { void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void onEndOfTranslationUnit() override; -private: - void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument, - const ASTContext &Context); +protected: + // Create diagnostics. These are virtual so that derived classes can change + // behaviour. + virtual void handleMoveFix(const ParmVarDecl &Param, + const DeclRefExpr &CopyArgument, + ASTContext &Context); + virtual void handleConstRefFix(const FunctionDecl &Function, + const ParmVarDecl &Param, ASTContext &Context); +private: ExprMutationAnalyzer::Memoized MutationAnalyzerCache; utils::IncludeInserter Inserter; const std::vector<StringRef> AllowedTypes; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
