[clang-tidy] Print type information to performance-unnecessary-* checks #152101
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Carlos Galvez (carlosgalvezp) ChangesUseful 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. Patch is 31.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152101.diff 11 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index b6f19811f5e5c..120f7fb749493 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -349,12 +349,13 @@ 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);
}
@@ -362,10 +363,11 @@ 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);
}
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index d89c3a69fc841..fbd2ba67805d9 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/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;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 85b31bc0b42a6..2dc5c73073cf8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-allowed-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-allowed-types.cpp
index a8de7cd1d8259..420f24cde4e66 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-allowed-types.cpp
+++ b/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();
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-excluded-container-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-excluded-container-types.cpp
index 5a02be656a799..1ebf8993e709c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization-excluded-container-types.cpp
+++ b/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();
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
index c0f1fb9c0f6d2..0168aeefc4583 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
+++ b/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;
}
-
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-allowed-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-allowed-types.cpp
index ff7aa23f7ff7c..1dd1404723195 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-allowed-types.cpp
+++ b/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) {
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp
index 99c2fe905bdf3..e09b85617d667 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-crash.cpp
+++ b/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);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-delayed.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-delayed.cpp
index 151b1cecf0f63..9df1d6f90ee38 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-delayed.cpp
+++ b/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-unn...
[truncated]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.