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+5chmi...@users.noreply.github.com> Date: Sat, 28 Oct 2023 18:39:18 +0200 Subject: [PATCH 1/4] [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 d77267588db9158..95e259873eb3c5a 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 794578ceeeba8fc..a4be41d20eae131 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 9af818be0415f39..bb042760d297a78 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 a94f857720b0357..50d0399ed4b0159 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+5chmi...@users.noreply.github.com> Date: Wed, 17 Jan 2024 02:12:37 +0100 Subject: [PATCH 2/4] 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 50d0399ed4b0159..22f91665c2ffdfe 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+5chmi...@users.noreply.github.com> Date: Wed, 17 Jan 2024 13:42:15 +0100 Subject: [PATCH 3/4] 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 a4be41d20eae131..9da468128743e90 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 22f91665c2ffdfe..f58ce4aebcbfc83 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 >From e0d97261decdba6b4fcdd8fbe1bc488e61d0ddd7 Mon Sep 17 00:00:00 2001 From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com> Date: Wed, 17 Jan 2024 21:50:29 +0100 Subject: [PATCH 4/4] better change description in release note --- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 95e259873eb3c5a..d963879f22b13c9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -382,8 +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. The check no longer emits a diagnostic for - variables used as the initializer of C++17 fold expressions. + 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits