[clang] [clang-tools-extra] [clang-tidy] fix misc-const-correctnes false-positive for fold expressions (PR #78320)
Julian Schmidt via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 17 07:09:22 PST 2024
https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/78320
>From 1951630fd6a0edc5258f5a775c95b9e9c30106df Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Sat, 28 Oct 2023 18:39:18 +0200
Subject: [PATCH 1/3] [clang-tidy] fix misc-const-correctnes false-positive for
fold expressions
The check no longer emits a diagnostic for variables used as the
initializer of 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
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++--
.../misc/const-correctness-templates.cpp | 17 +++++++++++++++++
clang/lib/Analysis/ExprMutationAnalyzer.cpp | 4 ++++
.../Analysis/ExprMutationAnalyzerTest.cpp | 15 +++++++++++++++
4 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d77267588db915..95e259873eb3c5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -382,7 +382,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
+ variables used as the initializer of C++17 fold expressions.
- Improved :doc:`misc-include-cleaner
<clang-tidy/checks/misc/include-cleaner>` check by adding option
@@ -502,7 +503,7 @@ Changes in existing checks
<clang-tidy/checks/readability/implicit-bool-conversion>` check to take
do-while loops into account for the `AllowIntegerConditions` and
`AllowPointerConditions` options. It also now provides more consistent
- suggestions when parentheses are added to the return value or expressions.
+ suggestions when parentheses are added to the return value or expressions.
It also ignores false-positives for comparison containing bool bitfield.
- Improved :doc:`readability-misleading-indentation
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..a4be41d20eae13 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,20 @@ 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 T>
+Stream& operator<<(Stream&, T);
+template <typename... Args>
+void concatenate(Args... args)
+{
+ Stream stream;
+ (stream << ... << args);
+ (args << ... << stream);
+}
+} // 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 different 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..50d0399ed4b015 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -359,6 +359,21 @@ 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.
+ const auto AST = buildASTFromCode(
+ "struct Stream {};"
+ "template <typename T> Stream& operator<<(Stream&, T); "
+ "template <typename... Args> void concatenate(Args... args) "
+ "{ Stream x; (x << ... << args); }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x << ... << args)"));
+}
+
// Section: expression as call argument
TEST(ExprMutationAnalyzerTest, ByValueArgument) {
>From 665bc668ec2108ed48c9add432871a5f209c15aa Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Wed, 17 Jan 2024 02:12:37 +0100
Subject: [PATCH 2/3] use -fno-delayed-template-parsing in added test in
ExprMutationAnalyzerTest
---
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index 50d0399ed4b015..22f91665c2ffdf 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -364,11 +364,12 @@ TEST(ExprMutationAnalyzerTest, FoldExpression) {
// 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.
- const auto AST = buildASTFromCode(
+ const auto AST = buildASTFromCodeWithArgs(
"struct Stream {};"
"template <typename T> Stream& operator<<(Stream&, T); "
"template <typename... Args> void concatenate(Args... args) "
- "{ Stream x; (x << ... << args); }");
+ "{ Stream x; (x << ... << args); }",
+ {"-fno-delayed-template-parsing"});
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x << ... << args)"));
>From e000a13607dec27b15c66f5f724a0287c553c956 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmidti at users.noreply.github.com>
Date: Wed, 17 Jan 2024 13:42:15 +0100
Subject: [PATCH 3/3] improve and fix tests
---
.../misc/const-correctness-templates.cpp | 17 ++++++++++++---
.../Analysis/ExprMutationAnalyzerTest.cpp | 21 ++++++++++++++++---
2 files changed, 32 insertions(+), 6 deletions(-)
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 a4be41d20eae13..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
@@ -37,13 +37,24 @@ namespace gh70323{
// operator is type dependent due to the parameter pack.
struct Stream {};
-template <typename T>
-Stream& operator<<(Stream&, T);
template <typename... Args>
-void concatenate(Args... 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/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index 22f91665c2ffdf..f58ce4aebcbfc8 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -364,15 +364,30 @@ TEST(ExprMutationAnalyzerTest, FoldExpression) {
// 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.
- const auto AST = buildASTFromCodeWithArgs(
+ auto AST = buildASTFromCodeWithArgs(
"struct Stream {};"
- "template <typename T> Stream& operator<<(Stream&, T); "
"template <typename... Args> void concatenate(Args... args) "
"{ Stream x; (x << ... << args); }",
{"-fno-delayed-template-parsing"});
- const auto Results =
+ 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
More information about the cfe-commits
mailing list