Skip to content

[clang-tidy] Print type information to performance-unnecessary-* checks #152101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -349,23 +349,25 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
const CheckContext &Ctx) {
auto Diagnostic =
diag(Ctx.Var.getLocation(),
"the %select{|const qualified }0variable %1 is "
"the %select{|const qualified }0variable %1 of type %2 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;
"reference|}0| but is never used}3; consider "
"%select{making it a const reference|removing the statement}3")
<< Ctx.Var.getType().isConstQualified() << &Ctx.Var << Ctx.Var.getType()
<< 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;
"local copy %0 of the variable %1 of type %2 is never "
"modified%select{"
"| and never used}3; consider %select{avoiding the copy|removing "
"the statement}3")
<< &Ctx.Var << &OldVar << Ctx.Var.getType() << Ctx.IsVarUnused;
maybeIssueFixes(Ctx, Diagnostic);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,12 @@ void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function,

auto Diag =
diag(Param.getLocation(),
"the %select{|const qualified }0parameter %1 is copied for each "
"the %select{|const qualified }0parameter %1 of type %2 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)
<< Param.getType();
// 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
Expand All @@ -173,10 +175,11 @@ void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function,
void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Param,
const DeclRefExpr &CopyArgument,
ASTContext &Context) {
auto Diag = diag(CopyArgument.getBeginLoc(),
"parameter %0 is passed by value and only copied once; "
"consider moving it to avoid unnecessary copies")
<< &Param;
auto Diag =
diag(CopyArgument.getBeginLoc(),
"parameter %0 of type %1 is passed by value and only copied once; "
"consider moving it to avoid unnecessary copies")
<< &Param << Param.getType();
// Do not propose fixes in macros since we cannot place them correctly.
if (CopyArgument.getBeginLoc().isMacroID())
return;
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ Changes in existing checks
when the format string is converted to a different type by an implicit
constructor call.

- Improved :doc:`performance-unnecessary-copy-initialization
<clang-tidy/checks/performance/unnecessary-copy-initialization>` by printing
the type of the diagnosed variable.

- Improved :doc:`performance-unnecessary-value-param
<clang-tidy/checks/performance/unnecessary-value-param>` by printing
the type of the diagnosed variable.

- Improved :doc:`portability-template-virtual-member-function
<clang-tidy/checks/portability/template-virtual-member-function>` check to
avoid false positives on pure virtual member functions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void negative_smart_ref() {

void positiveOtherType() {
const auto O = getOtherType();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' of type 'const OtherType' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const auto& O = getOtherType();
O.constMethod();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void positiveViewType() {
ExpensiveToCopy E;
ViewType<ExpensiveToCopy> V(E);
const auto O = V.view();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' of type 'ExpensiveToCopy const' is copy-constructed
// CHECK-FIXES: const auto& O = V.view();
O.constMethod();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void useByValue(ExpensiveToCopyType);

void PositiveFunctionCall() {
const auto AutoAssigned = ExpensiveTypeReference();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' of type 'const ExpensiveToCopyType' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
AutoAssigned.constMethod();

Expand All @@ -104,7 +104,7 @@ void PositiveFunctionCall() {

void PositiveStaticMethodCall() {
const auto AutoAssigned = ExpensiveToCopyType::instance();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' of type 'const ExpensiveToCopyType' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const auto& AutoAssigned = ExpensiveToCopyType::instance();
AutoAssigned.constMethod();

Expand Down Expand Up @@ -339,7 +339,7 @@ void NegativeStaticLocalVar(const ExpensiveToCopyType &Obj) {

void PositiveFunctionCallExpensiveTypeNonConstVariable() {
auto AutoAssigned = ExpensiveTypeReference();
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoAssigned' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoAssigned' of type 'ExpensiveToCopyType' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
AutoAssigned.constMethod();

Expand Down Expand Up @@ -472,13 +472,13 @@ struct NegativeConstructor {
// CHECK-FIXES: auto AssignedInMacro = T.reference();

UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' of type 'ExpensiveToCopyType' is copy-constructed

#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) ARGUMENT

void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
// CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
// CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' of type 'ExpensiveToCopyType' is copy-constructed
// Ensure fix is not applied.
// CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
CopyInMacroArg.constMethod();
Expand All @@ -487,7 +487,7 @@ void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
void PositiveLocalCopyConstMethodInvoked() {
ExpensiveToCopyType orig;
ExpensiveToCopyType copy_1 = orig;
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' of type 'ExpensiveToCopyType' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
// CHECK-FIXES: const ExpensiveToCopyType& copy_1 = orig;
copy_1.constMethod();
orig.constMethod();
Expand Down Expand Up @@ -599,7 +599,7 @@ void NegativeLocalCopyWeirdNonCopy() {
void WarningOnlyMultiDeclStmt() {
ExpensiveToCopyType orig;
ExpensiveToCopyType copy = orig, copy2;
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy' of the variable 'orig' of type 'ExpensiveToCopyType' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
// CHECK-FIXES: ExpensiveToCopyType copy = orig, copy2;
copy.constMethod();
}
Expand Down Expand Up @@ -676,7 +676,7 @@ struct function {

void positiveFakeStdFunction(std::function<void(int)> F) {
auto Copy = F;
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: local copy 'Copy' of the variable 'F' is never modified;
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: local copy 'Copy' of the variable 'F' of type 'std::function<void (int)>' is never modified;
// CHECK-FIXES: const auto& Copy = F;
Copy.constMethod();
}
Expand All @@ -687,7 +687,7 @@ void positiveInvokedOnStdFunction(
std::function<void(const ExpensiveToCopyType &)> Update,
const ExpensiveToCopyType Orig) {
auto Copy = Orig.reference();
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'Copy' is copy-constructed from a const reference
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'Copy' of type 'ExpensiveToCopyType' is copy-constructed from a const reference
// CHECK-FIXES: const auto& Copy = Orig.reference();
Update(Copy);
}
Expand Down Expand Up @@ -746,7 +746,7 @@ void positiveCopiedFromGetterOfReferenceToConstVar() {
ExpensiveToCopyType Orig;
const auto &Ref = Orig.reference();
auto UnnecessaryCopy = Ref.reference();
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'UnnecessaryCopy' is
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'UnnecessaryCopy' of type 'ExpensiveToCopyType' is
// CHECK-FIXES: const auto& UnnecessaryCopy = Ref.reference();
Orig.constMethod();
UnnecessaryCopy.constMethod();
Expand All @@ -755,18 +755,18 @@ void positiveCopiedFromGetterOfReferenceToConstVar() {
void positiveUnusedReferenceIsRemoved() {
// clang-format off
const auto AutoAssigned = ExpensiveTypeReference(); int i = 0; // Foo bar.
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference but is never used; consider removing the statement [performance-unnecessary-copy-initialization]
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' of type 'const ExpensiveToCopyType' is copy-constructed from a const reference but is never used; consider removing the statement [performance-unnecessary-copy-initialization]
// CHECK-FIXES-NOT: const auto AutoAssigned = ExpensiveTypeReference();
// CHECK-FIXES: int i = 0; // Foo bar.
auto TrailingCommentRemoved = ExpensiveTypeReference(); // Trailing comment.
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'TrailingCommentRemoved' is copy-constructed from a const reference but is never used;
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'TrailingCommentRemoved' of type 'ExpensiveToCopyType' is copy-constructed from a const reference but is never used;
// CHECK-FIXES-NOT: auto TrailingCommentRemoved = ExpensiveTypeReference();
// CHECK-FIXES-NOT: // Trailing comment.
// clang-format on

auto UnusedAndUnnecessary = ExpensiveTypeReference();
// Comments on a new line should not be deleted.
// CHECK-MESSAGES: [[@LINE-2]]:8: warning: the variable 'UnusedAndUnnecessary' is copy-constructed
// CHECK-MESSAGES: [[@LINE-2]]:8: warning: the variable 'UnusedAndUnnecessary' of type 'ExpensiveToCopyType' is copy-constructed
// CHECK-FIXES-NOT: auto UnusedAndUnnecessary = ExpensiveTypeReference();
// CHECK-FIXES: // Comments on a new line should not be deleted.
}
Expand Down Expand Up @@ -865,7 +865,7 @@ void negativeTemplateTypes() {

// Non-dependent types in template still trigger the check.
const auto UnnecessaryCopy = ExpensiveTypeReference();
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'UnnecessaryCopy' is copy-constructed
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'UnnecessaryCopy' of type 'const ExpensiveToCopyType' is copy-constructed
// CHECK-FIXES: const auto& UnnecessaryCopy = ExpensiveTypeReference();
UnnecessaryCopy.constMethod();
}
Expand All @@ -880,17 +880,17 @@ template <typename A>
void positiveSingleTemplateType() {
A Orig;
A SingleTmplParmTypeCopy = Orig;
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: local copy 'SingleTmplParmTypeCopy' of the variable 'Orig' is never modified
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: local copy 'SingleTmplParmTypeCopy' of the variable 'Orig' of type 'ExpensiveToCopyType' is never modified
// CHECK-FIXES: const A& SingleTmplParmTypeCopy = Orig;
SingleTmplParmTypeCopy.constMethod();

A UnnecessaryCopy2 = templatedReference<A>();
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the variable 'UnnecessaryCopy2' is copy-constructed from a const reference
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the variable 'UnnecessaryCopy2' of type 'ExpensiveToCopyType' is copy-constructed from a const reference
// CHECK-FIXES: const A& UnnecessaryCopy2 = templatedReference<A>();
UnnecessaryCopy2.constMethod();

A UnnecessaryCopy3 = Orig.template templatedAccessor<A>();
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the variable 'UnnecessaryCopy3' is copy-constructed from a const reference
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the variable 'UnnecessaryCopy3' of type 'ExpensiveToCopyType' is copy-constructed from a const reference
// CHECK-FIXES: const A& UnnecessaryCopy3 = Orig.template templatedAccessor<A>();
UnnecessaryCopy3.constMethod();
}
Expand Down Expand Up @@ -938,4 +938,3 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
ExpensiveToCopyType a2 = a1;
return a1 == t;
}

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void negative_smart_ref(smart_ref r) {
}

void positiveOtherType(OtherType O) {
// CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' of type 'OtherType' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-FIXES: void positiveOtherType(const OtherType& O) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct C
{
A a;
C(B, int i) : a(i) {}
// CHECK-MESSAGES: [[@LINE-1]]:6: warning: the parameter #1 is copied for each invocation but only used as a const reference; consider making it a const reference
// CHECK-MESSAGES: [[@LINE-1]]:6: warning: the parameter #1 of type 'B' is copied for each invocation but only used as a const reference; consider making it a const reference
};

C c(B(), 0);
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ class SomewhatTrivial {
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
// CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
// CHECK-MESSAGES: [[@LINE-1]]:60: warning: the const qualified parameter 'Obj' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-1]]:60: warning: the const qualified parameter 'Obj' of type 'const ExpensiveToCopyType' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
// CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj) {
}

void positiveExpensiveValue(ExpensiveToCopyType Obj);
// CHECK-FIXES: void positiveExpensiveValue(const ExpensiveToCopyType& Obj);
void positiveExpensiveValue(ExpensiveToCopyType Obj) {
// CHECK-MESSAGES: [[@LINE-1]]:49: warning: the parameter 'Obj' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-1]]:49: warning: the parameter 'Obj' of type 'ExpensiveToCopyType' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-FIXES: void positiveExpensiveValue(const ExpensiveToCopyType& Obj) {
Obj.constReference();
useAsConstReference(Obj);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@


int f1(int n, ABC v1, ABC v2) {
// CHECK-MESSAGES: [[@LINE-1]]:19: warning: the parameter 'v1' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-2]]:27: warning: the parameter 'v2' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-1]]:19: warning: the parameter 'v1' of type 'ABC' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-2]]:27: warning: the parameter 'v2' of type 'ABC' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-FIXES: int f1(int n, const ABC& v1, const ABC& v2) {
return v1.get(n) + v2.get(n);
}
void f2(int n, ABC v2) {
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: the parameter 'v2' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: the parameter 'v2' of type 'ABC' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
// CHECK-FIXES: void f2(int n, const ABC& v2) {
}
Loading