Skip to content

Commit

Permalink
Allow unnecessary-value-param to match templated functions including …
Browse files Browse the repository at this point in the history
…lambdas with auto.

Clang-Tidy unnecessary-value-param value param will be triggered for
templated functions if at least one instantiontion with expensive to
copy type is present in translation unit.

It is relatively common mistake to write lambda functions with auto
arguments for expensive to copy types.

Example:
Copy of the vectors will happen on every comparison.

void SortBySize(std::vector<std::vector<int>>& a) {
  std::sort(
      a.begin(), a.end(),
      [](auto x, auto y) { return a.size() < b.size()});
}
  • Loading branch information
%username% committed Jul 4, 2024
1 parent 1cf4340 commit 593b336
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
has(typeLoc(forEach(ExpensiveValueParamDecl))),
unless(isInstantiated()), decl().bind("functionDecl"))),
decl().bind("functionDecl"))),
this);
}

Expand Down Expand Up @@ -133,12 +133,9 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
// 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 a primary template or an explicit template
// specialization.
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
(Function->getTemplatedKind() != FunctionDecl::TK_NonTemplate))
isReferencedOutsideOfCallExpr(*Function, *Result.Context))
return;
for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
FunctionDecl = FunctionDecl->getPreviousDecl()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,19 +107,6 @@ struct PositiveConstValueConstructor {
// CHECK-FIXES: PositiveConstValueConstructor(const ExpensiveToCopyType& ConstCopy) {}
};

template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType S, T V) {
// CHECK-MESSAGES: [[@LINE-1]]:90: warning: the const qualified parameter 'S'
// CHECK-FIXES-NOT: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
}

void instantiated() {
templateWithNonTemplatizedParameter(ExpensiveToCopyType(), ExpensiveToCopyType());
templateWithNonTemplatizedParameter(ExpensiveToCopyType(), 5);
}

template <typename T> void negativeTemplateType(const T V) {
}

void negativeArray(const ExpensiveToCopyType[]) {
}

Expand Down Expand Up @@ -370,35 +357,3 @@ void fun() {
ExpensiveToCopyType E;
NegativeUsingConstructor S(E);
}

template<typename T>
void templateFunction(T) {
}

template<>
void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
// CHECK-MESSAGES: [[@LINE-1]]:64: warning: the parameter 'E' is copied
// CHECK-FIXES: void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
E.constReference();
}

template <class T>
T templateSpecializationFunction(ExpensiveToCopyType E) {
// CHECK-MESSAGES: [[@LINE-1]]:54: warning: the parameter 'E' is copied
// CHECK-FIXES-NOT: T templateSpecializationFunction(const ExpensiveToCopyType& E) {
return T();
}

template <>
bool templateSpecializationFunction(ExpensiveToCopyType E) {
// CHECK-MESSAGES: [[@LINE-1]]:57: warning: the parameter 'E' is copied
// CHECK-FIXES-NOT: bool templateSpecializationFunction(const ExpensiveToCopyType& E) {
return true;
}

template <>
int templateSpecializationFunction(ExpensiveToCopyType E) {
// CHECK-MESSAGES: [[@LINE-1]]:56: warning: the parameter 'E' is copied
// CHECK-FIXES-NOT: int templateSpecializationFunction(const ExpensiveToCopyType& E) {
return 0;
}

0 comments on commit 593b336

Please sign in to comment.