Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 1 | //===--- UnnecessaryValueParamCheck.cpp - clang-tidy-----------------------===// |
| 2 | // |
| 3 | // The LLVM Compiler Infrastructure |
| 4 | // |
| 5 | // This file is distributed under the University of Illinois Open Source |
| 6 | // License. See LICENSE.TXT for details. |
| 7 | // |
| 8 | //===----------------------------------------------------------------------===// |
| 9 | |
| 10 | #include "UnnecessaryValueParamCheck.h" |
| 11 | |
| 12 | #include "../utils/DeclRefExprUtils.h" |
| 13 | #include "../utils/FixItHintUtils.h" |
| 14 | #include "../utils/Matchers.h" |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 15 | #include "../utils/TypeTraits.h" |
| 16 | #include "clang/Frontend/CompilerInstance.h" |
| 17 | #include "clang/Lex/Lexer.h" |
| 18 | #include "clang/Lex/Preprocessor.h" |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 19 | |
| 20 | using namespace clang::ast_matchers; |
| 21 | |
| 22 | namespace clang { |
| 23 | namespace tidy { |
| 24 | namespace performance { |
| 25 | |
| 26 | namespace { |
| 27 | |
| 28 | std::string paramNameOrIndex(StringRef Name, size_t Index) { |
| 29 | return (Name.empty() ? llvm::Twine('#') + llvm::Twine(Index + 1) |
| 30 | : llvm::Twine('\'') + Name + llvm::Twine('\'')) |
| 31 | .str(); |
| 32 | } |
| 33 | |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 34 | template <typename S> |
| 35 | bool isSubset(const S &SubsetCandidate, const S &SupersetCandidate) { |
| 36 | for (const auto &E : SubsetCandidate) |
| 37 | if (SupersetCandidate.count(E) == 0) |
| 38 | return false; |
| 39 | return true; |
| 40 | } |
| 41 | |
Felix Berger | 85f9e8b3 | 2016-11-10 01:28:22 +0000 | [diff] [blame] | 42 | bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function, |
| 43 | ASTContext &Context) { |
| 44 | auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))), |
| 45 | unless(hasAncestor(callExpr()))), |
| 46 | Context); |
| 47 | return !Matches.empty(); |
| 48 | } |
| 49 | |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 50 | bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl, |
Felix Berger | 519de4b | 2016-12-16 02:47:56 +0000 | [diff] [blame] | 51 | ASTContext &Context) { |
| 52 | auto Matches = |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 53 | match(decl(forEachDescendant(declRefExpr( |
Felix Berger | 519de4b | 2016-12-16 02:47:56 +0000 | [diff] [blame] | 54 | equalsNode(&DeclRef), |
| 55 | unless(hasAncestor(stmt(anyOf(forStmt(), cxxForRangeStmt(), |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 56 | whileStmt(), doStmt()))))))), |
| 57 | Decl, Context); |
Felix Berger | 519de4b | 2016-12-16 02:47:56 +0000 | [diff] [blame] | 58 | return Matches.empty(); |
| 59 | } |
| 60 | |
Felix Berger | 603ea2d | 2017-07-26 00:45:41 +0000 | [diff] [blame] | 61 | bool isExplicitTemplateSpecialization(const FunctionDecl &Function) { |
| 62 | if (const auto *SpecializationInfo = Function.getTemplateSpecializationInfo()) |
| 63 | if (SpecializationInfo->getTemplateSpecializationKind() == |
| 64 | TSK_ExplicitSpecialization) |
| 65 | return true; |
| 66 | if (const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function)) |
| 67 | if (Method->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization && |
| 68 | Method->getMemberSpecializationInfo()->isExplicitSpecialization()) |
| 69 | return true; |
| 70 | return false; |
| 71 | } |
| 72 | |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 73 | } // namespace |
| 74 | |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 75 | UnnecessaryValueParamCheck::UnnecessaryValueParamCheck( |
| 76 | StringRef Name, ClangTidyContext *Context) |
| 77 | : ClangTidyCheck(Name, Context), |
| 78 | IncludeStyle(utils::IncludeSorter::parseIncludeStyle( |
Alexander Kornienko | b1c7432 | 2017-07-20 12:02:03 +0000 | [diff] [blame] | 79 | Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {} |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 80 | |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 81 | void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) { |
Ben Hamilton | b4ab4b6 | 2018-02-02 15:34:33 +0000 | [diff] [blame] | 82 | // This check is specific to C++ and doesn't apply to languages like |
| 83 | // Objective-C. |
| 84 | if (!getLangOpts().CPlusPlus) |
| 85 | return; |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 86 | const auto ExpensiveValueParamDecl = |
Alexander Kornienko | b215d47 | 2017-05-16 17:28:17 +0000 | [diff] [blame] | 87 | parmVarDecl(hasType(hasCanonicalType(allOf( |
| 88 | unless(referenceType()), matchers::isExpensiveToCopy()))), |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 89 | decl().bind("param")); |
| 90 | Finder->addMatcher( |
Alexander Kornienko | b215d47 | 2017-05-16 17:28:17 +0000 | [diff] [blame] | 91 | functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()), |
Felix Berger | e4ab060 | 2016-12-02 14:44:16 +0000 | [diff] [blame] | 92 | unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))), |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 93 | has(typeLoc(forEach(ExpensiveValueParamDecl))), |
Alexander Kornienko | b215d47 | 2017-05-16 17:28:17 +0000 | [diff] [blame] | 94 | unless(isInstantiated()), decl().bind("functionDecl")), |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 95 | this); |
| 96 | } |
| 97 | |
| 98 | void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) { |
| 99 | const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param"); |
| 100 | const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl"); |
| 101 | const size_t Index = std::find(Function->parameters().begin(), |
| 102 | Function->parameters().end(), Param) - |
| 103 | Function->parameters().begin(); |
| 104 | bool IsConstQualified = |
| 105 | Param->getType().getCanonicalType().isConstQualified(); |
| 106 | |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 107 | auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs( |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 108 | *Param, *Function, *Result.Context); |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 109 | auto ConstDeclRefExprs = utils::decl_ref_expr::constReferenceDeclRefExprs( |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 110 | *Param, *Function, *Result.Context); |
| 111 | |
| 112 | // Do not trigger on non-const value parameters when they are not only used as |
| 113 | // const. |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 114 | if (!isSubset(AllDeclRefExprs, ConstDeclRefExprs)) |
| 115 | return; |
| 116 | |
| 117 | // If the parameter is non-const, check if it has a move constructor and is |
| 118 | // only referenced once to copy-construct another object or whether it has a |
| 119 | // move assignment operator and is only referenced once when copy-assigned. |
| 120 | // In this case wrap DeclRefExpr with std::move() to avoid the unnecessary |
| 121 | // copy. |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 122 | if (!IsConstQualified && AllDeclRefExprs.size() == 1) { |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 123 | auto CanonicalType = Param->getType().getCanonicalType(); |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 124 | const auto &DeclRefExpr = **AllDeclRefExprs.begin(); |
| 125 | |
| 126 | if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) && |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 127 | ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) && |
| 128 | utils::decl_ref_expr::isCopyConstructorArgument( |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 129 | DeclRefExpr, *Function, *Result.Context)) || |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 130 | (utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) && |
| 131 | utils::decl_ref_expr::isCopyAssignmentArgument( |
Malcolm Parsons | cfa7d37 | 2017-01-03 12:10:44 +0000 | [diff] [blame] | 132 | DeclRefExpr, *Function, *Result.Context)))) { |
| 133 | handleMoveFix(*Param, DeclRefExpr, *Result.Context); |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 134 | return; |
| 135 | } |
| 136 | } |
| 137 | |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 138 | auto Diag = |
| 139 | diag(Param->getLocation(), |
| 140 | IsConstQualified ? "the const qualified parameter %0 is " |
| 141 | "copied for each invocation; consider " |
| 142 | "making it a reference" |
| 143 | : "the parameter %0 is copied for each " |
| 144 | "invocation but only used as a const reference; " |
| 145 | "consider making it a const reference") |
| 146 | << paramNameOrIndex(Param->getName(), Index); |
Felix Berger | 85f9e8b3 | 2016-11-10 01:28:22 +0000 | [diff] [blame] | 147 | // Do not propose fixes when: |
| 148 | // 1. the ParmVarDecl is in a macro, since we cannot place them correctly |
| 149 | // 2. the function is virtual as it might break overrides |
| 150 | // 3. the function is referenced outside of a call expression within the |
| 151 | // compilation unit as the signature change could introduce build errors. |
Felix Berger | 603ea2d | 2017-07-26 00:45:41 +0000 | [diff] [blame] | 152 | // 4. the function is an explicit template specialization. |
Felix Berger | 17934da | 2016-07-05 14:40:44 +0000 | [diff] [blame] | 153 | const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function); |
Felix Berger | 85f9e8b3 | 2016-11-10 01:28:22 +0000 | [diff] [blame] | 154 | if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()) || |
Felix Berger | 603ea2d | 2017-07-26 00:45:41 +0000 | [diff] [blame] | 155 | isReferencedOutsideOfCallExpr(*Function, *Result.Context) || |
| 156 | isExplicitTemplateSpecialization(*Function)) |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 157 | return; |
| 158 | for (const auto *FunctionDecl = Function; FunctionDecl != nullptr; |
| 159 | FunctionDecl = FunctionDecl->getPreviousDecl()) { |
| 160 | const auto &CurrentParam = *FunctionDecl->getParamDecl(Index); |
Etienne Bergeron | 2a4c00f | 2016-05-03 02:54:05 +0000 | [diff] [blame] | 161 | Diag << utils::fixit::changeVarDeclToReference(CurrentParam, |
Mandeep Singh Grang | 7c7ea7d | 2016-11-08 07:50:19 +0000 | [diff] [blame] | 162 | *Result.Context); |
Felix Berger | 7c6d289 | 2016-11-04 20:51:31 +0000 | [diff] [blame] | 163 | // The parameter of each declaration needs to be checked individually as to |
| 164 | // whether it is const or not as constness can differ between definition and |
| 165 | // declaration. |
| 166 | if (!CurrentParam.getType().getCanonicalType().isConstQualified()) |
Etienne Bergeron | 2a4c00f | 2016-05-03 02:54:05 +0000 | [diff] [blame] | 167 | Diag << utils::fixit::changeVarDeclToConst(CurrentParam); |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 168 | } |
| 169 | } |
| 170 | |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 171 | void UnnecessaryValueParamCheck::registerPPCallbacks( |
| 172 | CompilerInstance &Compiler) { |
| 173 | Inserter.reset(new utils::IncludeInserter( |
| 174 | Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle)); |
| 175 | Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); |
| 176 | } |
| 177 | |
| 178 | void UnnecessaryValueParamCheck::storeOptions( |
| 179 | ClangTidyOptions::OptionMap &Opts) { |
| 180 | Options.store(Opts, "IncludeStyle", |
| 181 | utils::IncludeSorter::toString(IncludeStyle)); |
| 182 | } |
| 183 | |
| 184 | void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var, |
| 185 | const DeclRefExpr &CopyArgument, |
| 186 | const ASTContext &Context) { |
| 187 | auto Diag = diag(CopyArgument.getLocStart(), |
| 188 | "parameter %0 is passed by value and only copied once; " |
| 189 | "consider moving it to avoid unnecessary copies") |
| 190 | << &Var; |
| 191 | // Do not propose fixes in macros since we cannot place them correctly. |
| 192 | if (CopyArgument.getLocStart().isMacroID()) |
| 193 | return; |
| 194 | const auto &SM = Context.getSourceManager(); |
Kirill Bobyrev | 11cea45 | 2016-08-01 12:06:18 +0000 | [diff] [blame] | 195 | auto EndLoc = Lexer::getLocForEndOfToken(CopyArgument.getLocation(), 0, SM, |
| 196 | Context.getLangOpts()); |
Felix Berger | 7f88275 | 2016-07-01 20:12:15 +0000 | [diff] [blame] | 197 | Diag << FixItHint::CreateInsertion(CopyArgument.getLocStart(), "std::move(") |
| 198 | << FixItHint::CreateInsertion(EndLoc, ")"); |
| 199 | if (auto IncludeFixit = Inserter->CreateIncludeInsertion( |
| 200 | SM.getFileID(CopyArgument.getLocStart()), "utility", |
| 201 | /*IsAngled=*/true)) |
| 202 | Diag << *IncludeFixit; |
| 203 | } |
| 204 | |
Felix Berger | 3c8edde | 2016-03-29 02:42:38 +0000 | [diff] [blame] | 205 | } // namespace performance |
| 206 | } // namespace tidy |
| 207 | } // namespace clang |