https://github.com/5chmidti created https://github.com/llvm/llvm-project/pull/78320
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 >From d26970b63831d2554d5e9c6c73d41c147220d68d 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] [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 | 10 +++------- .../misc/const-correctness-templates.cpp | 17 +++++++++++++++++ clang/lib/Analysis/ExprMutationAnalyzer.cpp | 4 ++++ .../Analysis/ExprMutationAnalyzerTest.cpp | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d77267588db915..c975b99c5786be 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,12 +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. - It also ignores false-positives for comparison containing bool bitfield. - -- Improved :doc:`readability-misleading-indentation - <clang-tidy/checks/readability/misleading-indentation>` check to ignore - false-positives for line started with empty macro. + suggestions when parentheses are added to the return value. - Improved :doc:`readability-non-const-parameter <clang-tidy/checks/readability/non-const-parameter>` check to ignore 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits