[clang] 9f8ccf5 - [clang-tidy] fix misc-const-correctnes false-positive for fold expressions (#78320)

[via cfe-commits]

Author: Julian Schmidt
Date: 2024-01-22T21:51:39+01:00
New Revision: 9f8ccf50dde17adae1368a0cf41edadc8327aaf4

URL: https://github.com/llvm/llvm-project/commit/9f8ccf50dde17adae1368a0cf41edadc8327aaf4
DIFF: https://github.com/llvm/llvm-project/commit/9f8ccf50dde17adae1368a0cf41edadc8327aaf4.diff

LOG: [clang-tidy] fix misc-const-correctnes false-positive for fold expressions (#78320)

The check no longer emits a diagnostic for non-parameter-pack 
variables in C++17 fold expressions.
The operator used is type-dependent because of the parameter pack
and can therefore not be guaranteed to not mutate the variable.

Fixes: #70323

Added: 
    

Modified: 
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
    clang/lib/Analysis/ExprMutationAnalyzer.cpp
    clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1015b6bd188d8c..27f098a54327dc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -404,7 +404,8 @@ Changes in existing checks
   using pointer to member function. Additionally, the check no longer emits
   a diagnostic when a variable that is not type-dependent is an operand of a
   type-dependent binary operator. Improved performance of the check through
-  optimizations.
+  optimizations. The check no longer emits a diagnostic for non-parameter-pack
+  variables in C++17 fold expressions.
 
 - Improved :doc:`misc-include-cleaner
   <clang-tidy/checks/misc/include-cleaner>` check by adding option

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
index 794578ceeeba8f..9da468128743e9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
@@ -30,3 +30,31 @@ namespace gh57297{
 struct Stream { };
 template <typename T> void f() { T t; Stream x; x << t; }
 } // namespace gh57297
+
+namespace gh70323{
+// A fold expression may contain the checked variable as it's initializer.
+// We don't know if the operator modifies that variable because the
+// operator is type dependent due to the parameter pack.
+
+struct Stream {};
+template <typename... Args>
+void concatenate1(Args... args)
+{
+    Stream stream;
+    (stream << ... << args);
+}
+
+template <typename... Args>
+void concatenate2(Args... args)
+{
+    Stream stream;
+    (args << ... << stream);
+}
+
+template <typename... Args>
+void concatenate3(Args... args)
+{
+    Stream stream;
+    (..., (stream << args));
+}
+} // namespace gh70323

diff  --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 9af818be0415f3..bb042760d297a7 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -343,6 +343,10 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
       // in 
diff erent instantiations of the template.
       binaryOperator(isTypeDependent(),
                      hasEitherOperand(ignoringImpCasts(canResolveToExpr(Exp)))),
+      // A fold expression may contain `Exp` as it's initializer.
+      // We don't know if the operator modifies `Exp` because the
+      // operator is type dependent due to the parameter pack.
+      cxxFoldExpr(hasFoldInit(ignoringImpCasts(canResolveToExpr(Exp)))),
       // Within class templates and member functions the member expression might
       // not be resolved. In that case, the `callExpr` is considered to be a
       // modification.

diff  --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index a94f857720b035..f58ce4aebcbfc8 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -359,6 +359,37 @@ TEST(ExprMutationAnalyzerTest, DependentOperatorWithNonDependentOperand) {
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << t"));
 }
 
+TEST(ExprMutationAnalyzerTest, FoldExpression) {
+  // gh70323
+  // A fold expression may contain `Exp` as it's initializer.
+  // We don't know if the operator modifies `Exp` because the
+  // operator is type dependent due to the parameter pack.
+  auto AST = buildASTFromCodeWithArgs(
+      "struct Stream {};"
+      "template <typename... Args> void concatenate(Args... args) "
+      "{ Stream x; (x << ... << args); }",
+      {"-fno-delayed-template-parsing"});
+  auto Results =
+      match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x << ... << args)"));
+
+  AST = buildASTFromCodeWithArgs(
+      "struct Stream {};"
+      "template <typename... Args> void concatenate(Args... args) "
+      "{ Stream x; (args << ... << x); }",
+      {"-fno-delayed-template-parsing"});
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(args << ... << x)"));
+
+  AST = buildASTFromCodeWithArgs(
+      "struct Stream {};"
+      "template <typename... Args> void concatenate(Args... args) "
+      "{ Stream x; (..., (x << args)); }",
+      {"-fno-delayed-template-parsing"});
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x << args"));
+}
+
 // Section: expression as call argument
 
 TEST(ExprMutationAnalyzerTest, ByValueArgument) {