2016-03-29 02:42:38 +00:00
|
|
|
//===--- UnnecessaryValueParamCheck.cpp - clang-tidy-----------------------===//
|
|
|
|
//
|
2019-01-19 08:50:56 +00:00
|
|
|
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
|
|
// See https://llvm.org/LICENSE.txt for license information.
|
|
|
|
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
2016-03-29 02:42:38 +00:00
|
|
|
//
|
|
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
|
|
|
|
#include "UnnecessaryValueParamCheck.h"
|
|
|
|
|
|
|
|
#include "../utils/DeclRefExprUtils.h"
|
|
|
|
#include "../utils/FixItHintUtils.h"
|
|
|
|
#include "../utils/Matchers.h"
|
2018-10-12 13:05:21 +00:00
|
|
|
#include "../utils/OptionsUtils.h"
|
2016-07-01 20:12:15 +00:00
|
|
|
#include "../utils/TypeTraits.h"
|
|
|
|
#include "clang/Frontend/CompilerInstance.h"
|
|
|
|
#include "clang/Lex/Lexer.h"
|
|
|
|
#include "clang/Lex/Preprocessor.h"
|
2023-01-07 20:02:20 -08:00
|
|
|
#include <optional>
|
2016-03-29 02:42:38 +00:00
|
|
|
|
|
|
|
using namespace clang::ast_matchers;
|
|
|
|
|
|
|
|
namespace clang {
|
|
|
|
namespace tidy {
|
|
|
|
namespace performance {
|
|
|
|
|
|
|
|
namespace {
|
|
|
|
|
|
|
|
std::string paramNameOrIndex(StringRef Name, size_t Index) {
|
|
|
|
return (Name.empty() ? llvm::Twine('#') + llvm::Twine(Index + 1)
|
|
|
|
: llvm::Twine('\'') + Name + llvm::Twine('\''))
|
|
|
|
.str();
|
|
|
|
}
|
|
|
|
|
2016-11-10 01:28:22 +00:00
|
|
|
bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
|
|
|
|
ASTContext &Context) {
|
|
|
|
auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
|
|
|
|
unless(hasAncestor(callExpr()))),
|
|
|
|
Context);
|
|
|
|
return !Matches.empty();
|
|
|
|
}
|
|
|
|
|
2017-01-03 12:10:44 +00:00
|
|
|
bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
|
2016-12-16 02:47:56 +00:00
|
|
|
ASTContext &Context) {
|
2019-11-12 15:15:56 +00:00
|
|
|
auto Matches = match(
|
2020-12-11 00:52:35 +01:00
|
|
|
traverse(TK_AsIs,
|
2019-11-12 15:15:56 +00:00
|
|
|
decl(forEachDescendant(declRefExpr(
|
|
|
|
equalsNode(&DeclRef),
|
|
|
|
unless(hasAncestor(stmt(anyOf(forStmt(), cxxForRangeStmt(),
|
|
|
|
whileStmt(), doStmt())))))))),
|
|
|
|
Decl, Context);
|
2016-12-16 02:47:56 +00:00
|
|
|
return Matches.empty();
|
|
|
|
}
|
|
|
|
|
2016-03-29 02:42:38 +00:00
|
|
|
} // namespace
|
|
|
|
|
2016-07-01 20:12:15 +00:00
|
|
|
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
|
|
|
|
StringRef Name, ClangTidyContext *Context)
|
|
|
|
: ClangTidyCheck(Name, Context),
|
2020-07-27 12:48:53 +01:00
|
|
|
Inserter(Options.getLocalOrGlobal("IncludeStyle",
|
[clang-tidy] Add a Standalone diagnostics mode to clang-tidy
Adds a flag to `ClangTidyContext` that is used to indicate to checks that fixes will only be applied one at a time.
This is to indicate to checks that each fix emitted should not depend on any other fixes emitted across the translation unit.
I've currently implemented the `IncludeInserter`, `LoopConvertCheck` and `PreferMemberInitializerCheck` to use these support these modes.
Reasoning behind this is in use cases like `clangd` it's only possible to apply one fix at a time.
For include inserter checks, the include is only added once for the first diagnostic that requires it, this will result in subsequent fixes not having the included needed.
A similar issue is seen in the `PreferMemberInitializerCheck` where the `:` will only be added for the first member that needs fixing.
Fixes emitted in `StandaloneDiagsMode` will likely result in malformed code if they are applied all together, conversely fixes currently emitted may result in malformed code if they are applied one at a time.
For this reason invoking `clang-tidy` from the binary will always with `StandaloneDiagsMode` disabled, However using it as a library its possible to select the mode you wish to use, `clangd` always selects `StandaloneDiagsMode`.
This is an example of the current behaviour failing
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E) {
A = D;
B = E; // Fix Here
}
};
```
Incorrectly transformed to:
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E), B(E) {
A = D;
// Fix Here
}
};
```
In `StandaloneDiagsMode`, it gets transformed to:
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E) : B(E) {
A = D;
// Fix Here
}
};
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D97121
2022-04-16 09:53:32 +01:00
|
|
|
utils::IncludeSorter::IS_LLVM),
|
|
|
|
areDiagsSelfContained()),
|
2018-10-12 13:05:21 +00:00
|
|
|
AllowedTypes(
|
|
|
|
utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
|
2016-07-01 20:12:15 +00:00
|
|
|
|
2016-03-29 02:42:38 +00:00
|
|
|
void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
|
2018-10-12 13:05:21 +00:00
|
|
|
const auto ExpensiveValueParamDecl = parmVarDecl(
|
2018-11-25 02:41:01 +00:00
|
|
|
hasType(qualType(
|
2018-10-12 13:05:21 +00:00
|
|
|
hasCanonicalType(matchers::isExpensiveToCopy()),
|
|
|
|
unless(anyOf(hasCanonicalType(referenceType()),
|
|
|
|
hasDeclaration(namedDecl(
|
2018-11-25 02:41:01 +00:00
|
|
|
matchers::matchesAnyListedName(AllowedTypes))))))),
|
2018-10-12 13:05:21 +00:00
|
|
|
decl().bind("param"));
|
2016-03-29 02:42:38 +00:00
|
|
|
Finder->addMatcher(
|
2019-11-12 15:15:56 +00:00
|
|
|
traverse(
|
2020-12-11 00:52:35 +01:00
|
|
|
TK_AsIs,
|
2019-11-12 15:15:56 +00:00
|
|
|
functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
|
|
|
|
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
|
|
|
|
has(typeLoc(forEach(ExpensiveValueParamDecl))),
|
|
|
|
unless(isInstantiated()), decl().bind("functionDecl"))),
|
2016-03-29 02:42:38 +00:00
|
|
|
this);
|
|
|
|
}
|
|
|
|
|
|
|
|
void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
|
|
|
|
const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
|
|
|
|
const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
|
|
|
|
|
2020-12-11 00:52:35 +01:00
|
|
|
TraversalKindScope RAII(*Result.Context, TK_AsIs);
|
2019-11-12 15:15:56 +00:00
|
|
|
|
2018-09-17 17:59:51 +00:00
|
|
|
FunctionParmMutationAnalyzer &Analyzer =
|
|
|
|
MutationAnalyzers.try_emplace(Function, *Function, *Result.Context)
|
|
|
|
.first->second;
|
|
|
|
if (Analyzer.isMutated(Param))
|
2016-07-01 20:12:15 +00:00
|
|
|
return;
|
2018-08-03 17:23:37 +00:00
|
|
|
|
|
|
|
const bool IsConstQualified =
|
|
|
|
Param->getType().getCanonicalType().isConstQualified();
|
2016-07-01 20:12:15 +00:00
|
|
|
|
|
|
|
// If the parameter is non-const, check if it has a move constructor and is
|
|
|
|
// only referenced once to copy-construct another object or whether it has a
|
|
|
|
// move assignment operator and is only referenced once when copy-assigned.
|
|
|
|
// In this case wrap DeclRefExpr with std::move() to avoid the unnecessary
|
|
|
|
// copy.
|
2018-08-03 17:23:37 +00:00
|
|
|
if (!IsConstQualified) {
|
|
|
|
auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs(
|
|
|
|
*Param, *Function, *Result.Context);
|
|
|
|
if (AllDeclRefExprs.size() == 1) {
|
|
|
|
auto CanonicalType = Param->getType().getCanonicalType();
|
|
|
|
const auto &DeclRefExpr = **AllDeclRefExprs.begin();
|
|
|
|
|
|
|
|
if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
|
|
|
|
((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
|
|
|
|
utils::decl_ref_expr::isCopyConstructorArgument(
|
|
|
|
DeclRefExpr, *Function, *Result.Context)) ||
|
|
|
|
(utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) &&
|
|
|
|
utils::decl_ref_expr::isCopyAssignmentArgument(
|
|
|
|
DeclRefExpr, *Function, *Result.Context)))) {
|
|
|
|
handleMoveFix(*Param, DeclRefExpr, *Result.Context);
|
|
|
|
return;
|
|
|
|
}
|
2016-07-01 20:12:15 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2022-10-30 09:41:55 -07:00
|
|
|
const size_t Index = llvm::find(Function->parameters(), Param) -
|
2018-08-03 17:23:37 +00:00
|
|
|
Function->parameters().begin();
|
|
|
|
|
2016-03-29 02:42:38 +00:00
|
|
|
auto Diag =
|
|
|
|
diag(Param->getLocation(),
|
2021-02-26 19:10:25 +00:00
|
|
|
"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);
|
2016-11-10 01:28:22 +00:00
|
|
|
// 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.
|
2022-05-30 09:43:27 +08:00
|
|
|
// 4. the function is a primary template or an explicit template
|
|
|
|
// specialization.
|
2016-07-05 14:40:44 +00:00
|
|
|
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
|
2018-08-09 22:42:26 +00:00
|
|
|
if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
|
2017-07-26 00:45:41 +00:00
|
|
|
isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
|
2022-05-30 09:43:27 +08:00
|
|
|
(Function->getTemplatedKind() != FunctionDecl::TK_NonTemplate))
|
2016-03-29 02:42:38 +00:00
|
|
|
return;
|
|
|
|
for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
|
|
|
|
FunctionDecl = FunctionDecl->getPreviousDecl()) {
|
|
|
|
const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
|
2016-05-03 02:54:05 +00:00
|
|
|
Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
|
2016-11-08 07:50:19 +00:00
|
|
|
*Result.Context);
|
2016-11-04 20:51:31 +00:00
|
|
|
// The parameter of each declaration needs to be checked individually as to
|
|
|
|
// whether it is const or not as constness can differ between definition and
|
|
|
|
// declaration.
|
[clang-tidy] implement utility-function to add 'const' to variables
Summary:
This patch extends the already existing facility to add 'const' to variables
to be more flexible and correct. The previous version did not consider pointers
as value AND pointee. For future automatic introduction for const-correctness
this shortcoming needs to be fixed.
It always allows configuration where the 'const' token is inserted, either on
the left side (if possible) or the right side.
It adds many unit-tests to the utility-function that did not exist before, as
the function was implicitly tested through clang-tidy checks. These
tests were not changed, as the API is still compatible.
Reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri
Reviewed By: aaron.ballman
Subscribers: jdoerfert, mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D54395
2020-01-03 20:36:49 +01:00
|
|
|
if (!CurrentParam.getType().getCanonicalType().isConstQualified()) {
|
|
|
|
if (llvm::Optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl(
|
|
|
|
CurrentParam, *Result.Context, DeclSpec::TQ::TQ_const))
|
|
|
|
Diag << *Fix;
|
|
|
|
}
|
2016-03-29 02:42:38 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2016-07-01 20:12:15 +00:00
|
|
|
void UnnecessaryValueParamCheck::registerPPCallbacks(
|
2019-03-22 18:58:12 +00:00
|
|
|
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
|
2020-07-27 12:48:53 +01:00
|
|
|
Inserter.registerPreprocessor(PP);
|
2016-07-01 20:12:15 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
void UnnecessaryValueParamCheck::storeOptions(
|
|
|
|
ClangTidyOptions::OptionMap &Opts) {
|
2020-07-27 12:48:53 +01:00
|
|
|
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
|
2018-10-12 13:05:21 +00:00
|
|
|
Options.store(Opts, "AllowedTypes",
|
|
|
|
utils::options::serializeStringList(AllowedTypes));
|
2016-07-01 20:12:15 +00:00
|
|
|
}
|
|
|
|
|
2018-09-17 17:59:51 +00:00
|
|
|
void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
|
|
|
|
MutationAnalyzers.clear();
|
|
|
|
}
|
|
|
|
|
2016-07-01 20:12:15 +00:00
|
|
|
void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
|
|
|
|
const DeclRefExpr &CopyArgument,
|
|
|
|
const ASTContext &Context) {
|
2018-08-09 22:42:26 +00:00
|
|
|
auto Diag = diag(CopyArgument.getBeginLoc(),
|
2016-07-01 20:12:15 +00:00
|
|
|
"parameter %0 is passed by value and only copied once; "
|
|
|
|
"consider moving it to avoid unnecessary copies")
|
|
|
|
<< &Var;
|
|
|
|
// Do not propose fixes in macros since we cannot place them correctly.
|
2018-08-09 22:42:26 +00:00
|
|
|
if (CopyArgument.getBeginLoc().isMacroID())
|
2016-07-01 20:12:15 +00:00
|
|
|
return;
|
|
|
|
const auto &SM = Context.getSourceManager();
|
2016-08-01 12:06:18 +00:00
|
|
|
auto EndLoc = Lexer::getLocForEndOfToken(CopyArgument.getLocation(), 0, SM,
|
|
|
|
Context.getLangOpts());
|
2018-08-09 22:42:26 +00:00
|
|
|
Diag << FixItHint::CreateInsertion(CopyArgument.getBeginLoc(), "std::move(")
|
2020-06-19 13:22:39 +01:00
|
|
|
<< FixItHint::CreateInsertion(EndLoc, ")")
|
2020-07-27 12:48:53 +01:00
|
|
|
<< Inserter.createIncludeInsertion(
|
2020-09-28 14:58:27 +02:00
|
|
|
SM.getFileID(CopyArgument.getBeginLoc()), "<utility>");
|
2016-07-01 20:12:15 +00:00
|
|
|
}
|
|
|
|
|
2016-03-29 02:42:38 +00:00
|
|
|
} // namespace performance
|
|
|
|
} // namespace tidy
|
|
|
|
} // namespace clang
|