[clang-tidy] Fix param-pack fix-its for 'performance-unnecessary-value-param' check

[vbvictor]

Closes #154755.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Baranov Victor (vbvictor)

Changes

Closes #154755.

---

Full diff: https://github.com/llvm/llvm-project/pull/164130.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp (+5)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
• (modified) clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp (+24)

diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
index 086c7f3a15d45..b30c83e3aeb35 100644
--- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -21,6 +21,11 @@ FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
   SourceLocation AmpLocation = Var.getLocation();
   auto Token = utils::lexer::getPreviousToken(
       AmpLocation, Context.getSourceManager(), Context.getLangOpts());
+
+  // For parameter packs the '&' must go before the '...' token
+  if (Token.is(tok::ellipsis))
+    return FixItHint::CreateInsertion(Token.getLocation(), "&");
+
   if (!Token.is(tok::unknown))
     AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
                                              Context.getSourceManager(),
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a94dd9737468c..2a4511b1c92d5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -384,7 +384,8 @@ Changes in existing checks
 
 - Improved :doc:`performance-unnecessary-value-param
   <clang-tidy/checks/performance/unnecessary-value-param>` by printing
-  the type of the diagnosed variable.
+  the type of the diagnosed variable and correctly generating fix-it hints for
+  parameter-pack arguments.
 
 - Improved :doc:`portability-template-virtual-member-function
   <clang-tidy/checks/portability/template-virtual-member-function>` check to
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp
index 688c79bbaa9ac..43a7df4da75fd 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp
@@ -96,3 +96,27 @@ void lambdaNonConstAutoValue() {
   };
   fn(ExpensiveToCopyType());
 }
+
+template <typename... Args>
+void ParameterPack(Args... args) {
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: the parameter 'args' of type 'ExpensiveToCopyType'
+  // CHECK-FIXES: void ParameterPack(const Args&... args) {
+}
+
+template <typename... Args>
+void ParameterPackWithParams(const ExpensiveToCopyType E1, ExpensiveToCopyType E2, Args... args) {
+  // CHECK-MESSAGES: [[@LINE-1]]:56: warning: the const qualified parameter 'E1'
+  // CHECK-MESSAGES: [[@LINE-2]]:80: warning: the parameter 'E2'
+  // CHECK-MESSAGES: [[@LINE-3]]:92: warning: the parameter 'args'
+  // CHECK-FIXES: void ParameterPackWithParams(const ExpensiveToCopyType& E1, const ExpensiveToCopyType& E2, const Args&... args) {
+}
+
+template <typename... Args>
+void PackWithNonExpensive(int x, Args... args) {}
+
+void instantiatedParameterPack() {
+  ExpensiveToCopyType E;
+  ParameterPack(E);
+  ParameterPackWithParams(E, E, E);
+  PackWithNonExpensive(5, 5);
+}