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

Useful when the check warns on template functions to know which type
it's complaining about. Otherwise, since the instantiation stack is not
printed, it's very hard to tell.

Co-authored-by: Carlos Gálvez <[email protected]>

Author: carlosgalvezp

clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

@@ -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);
 }
 

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

@@ -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
@@ -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;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -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.

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-allowed-types.cpp

@@ -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();
 }

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-excluded-container-types.cpp

@@ -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();
 }

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp

@@ -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();
 
@@ -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();
 
@@ -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();
 
@@ -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();
@@ -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();
@@ -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();
 }
@@ -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();
 }
@@ -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);
 }
@@ -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();
@@ -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.
 }
@@ -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();
 }
@@ -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();
 }
@@ -938,4 +938,3 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
   ExpensiveToCopyType a2 = a1;
   return a1 == t;
 }
-

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-allowed-types.cpp

@@ -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) {
 }
 

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp

@@ -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);

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-delayed.cpp

@@ -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);

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp

@@ -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) {
 }