https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/192895
>From 05d4e362ce8312beccab0b030c76c672fc70da7d Mon Sep 17 00:00:00 2001 From: Yuta Nakamura <[email protected]> Date: Mon, 18 May 2026 15:59:36 -0700 Subject: [PATCH 1/4] [clang-tidy] Fix false positive in bugprone-use-after-move for std::tie std::tie(a, b) = expr reinitializes all variables passed to std::tie because the tuple assignment operator writes back through the stored references. The check was not recognizing this pattern, causing a false positive on the second std::tie assignment in loops like: std::tie(a, b) = foo(std::move(a), std::move(b)); std::tie(a, b) = foo(std::move(a), std::move(b)); // false positive Add std::tie assignment as a reinitialization case in makeReinitMatcher(). Update documentation and tests accordingly. Fixes #136105. --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 11 +++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/bugprone/use-after-move.rst | 5 ++ .../checkers/bugprone/use-after-move.cpp | 90 +++++++++++++++++++ 4 files changed, 111 insertions(+) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 399442f52bd33..ab44b4be868ae 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -134,6 +134,17 @@ makeReinitMatcher(const ValueDecl *MovedVariable, // built-in types. binaryOperation(hasOperatorName("="), hasLHS(ignoringParenImpCasts(DeclRefMatcher))), + // std::tie() assignment: std::tie(a, b) = expr reinitializes + // all variables passed to std::tie because the tuple + // assignment writes back through the stored references. + // ignoringImplicit strips the MaterializeTemporaryExpr that + // Clang inserts when calling operator= on the prvalue tuple. + binaryOperation( + hasOperatorName("="), + hasLHS(ignoringImplicit( + callExpr(callee(functionDecl(hasName("::std::tie"))), + hasAnyArgument(ignoringParenImpCasts( + DeclRefMatcher)))))), // Declaration. We treat this as a type of reinitialization // too, so we don't need to treat it separately. declStmt(hasDescendant(equalsNode(MovedVariable))), diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 89fb1684bba7c..84215a5f4381e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -433,6 +433,11 @@ Changes in existing checks - Avoid false positives when moving object is reinitialized via the base class's ``operator=``. + - Fix a false positive when a moved-from variable is reinitialized + via a ``std::tie()`` assignment (e.g. ``std::tie(a, b) = f(std::move(a), + std::move(b))``). The tuple assignment writes back through the stored + references, which fully reinitializes the captured variables. + - Improved :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check by adding the `AllowExplicitObjectParameters` option. When enabled, diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst index da72f742b38d0..4e746b2633c07 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst @@ -233,6 +233,11 @@ The check considers a variable to be reinitialized in the following cases: - A member function marked with the ``[[clang::reinitializes]]`` attribute is called on the variable. + - The variable is passed as an argument to ``std::tie`` on the left-hand + side of an assignment (e.g. ``std::tie(a, b) = f(...)``). The tuple + assignment operator writes back through the stored references, which + reinitializes each named variable. + If the variable in question is a struct and an individual member variable of that struct is written to, the check does not consider this to be a reinitialization -- even if, eventually, all member variables of the struct are diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp index d4e78d359b654..0395650a82d1b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp @@ -17,6 +17,7 @@ #include <map> #include <set> #include <string> +#include <tuple> #include <unordered_map> #include <unordered_set> #include <utility> @@ -1814,3 +1815,92 @@ namespace GH62206 { (d) = b; // Should not warn } } // namespace GH62206 + + +std::pair<std::string, std::string> makeStringPair(std::string a, + std::string b); + +void stdTieIsReinit() { + std::string a, b; + std::move(a); + std::move(b); + std::tie(a, b) = makeStringPair("x", "y"); + a.size(); + b.size(); +} + +void stdTieWithIgnore() { + std::string a; + std::move(a); + std::tie(a, std::ignore) = makeStringPair("x", "y"); + a.size(); +} + +void stdTieIgnoreFlipped() { + std::string a; + std::move(a); + std::tie(std::ignore, a) = makeStringPair("x", "y"); + a.size(); +} + +void stdTieThreeVars() { + std::string a, b, c; + std::move(a); + std::move(b); + std::move(c); + std::tie(a, b, c) = + std::make_tuple(std::string("x"), std::string("y"), std::string("z")); + a.size(); + b.size(); + c.size(); +} + +void stdTiePartialReinit() { + std::string a, b, c; + std::move(a); + std::move(b); + std::move(c); + std::tie(b) = std::make_tuple(std::string("y")); + b.size(); + std::tie(c, b, std::ignore) = + std::make_tuple(std::string("x"), std::string("y"), std::string("z")); + b.size(); + c.size(); + a.size(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-11]]:3: note: move occurred here +} + +void stdTieInLoop() { + std::string a, b; + while (true) { + std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); + std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); + } +} + +template <typename T> +void stdTieReinitInTemplate() { + T a, b; + std::move(a); + std::move(b); + std::tie(a, b) = std::make_tuple(T(), T()); + a.size(); + b.size(); +} + +template <typename T> +void stdTiePartialReinitInTemplate() { + T a, b; + std::move(a); + std::tie(b) = std::make_tuple(T()); + b.size(); + a.size(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-5]]:3: note: move occurred here +} + +void callStdTieTemplateTests() { + stdTieReinitInTemplate<std::string>(); + stdTiePartialReinitInTemplate<std::string>(); +} >From 04181c90a5e12fe055e43e04c46417e49d9fc200 Mon Sep 17 00:00:00 2001 From: Yuta Nakamura <[email protected]> Date: Mon, 1 Jun 2026 14:05:13 -0400 Subject: [PATCH 2/4] [clang-tidy] Handle parenthesized std::tie in bugprone-use-after-move Add ignoringParenImpCasts to the std::tie reinit matcher so that parenthesized forms like (std::tie(a, b)) = expr are also recognized as reinitialization. Add test cases covering parenthesized std::tie with std::ignore and partial reinitialization. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 8 +- .../checkers/bugprone/use-after-move.cpp | 215 ++++++++++-------- 2 files changed, 131 insertions(+), 92 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index ab44b4be868ae..8dae9f6278459 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -138,13 +138,15 @@ makeReinitMatcher(const ValueDecl *MovedVariable, // all variables passed to std::tie because the tuple // assignment writes back through the stored references. // ignoringImplicit strips the MaterializeTemporaryExpr that - // Clang inserts when calling operator= on the prvalue tuple. + // Clang inserts when calling operator= on the prvalue tuple; + // ignoringParenImpCasts additionally handles parenthesized + // forms such as (std::tie(a, b)) = expr. binaryOperation( hasOperatorName("="), - hasLHS(ignoringImplicit( + hasLHS(ignoringImplicit(ignoringParenImpCasts( callExpr(callee(functionDecl(hasName("::std::tie"))), hasAnyArgument(ignoringParenImpCasts( - DeclRefMatcher)))))), + DeclRefMatcher))))))), // Declaration. We treat this as a type of reinitialization // too, so we don't need to treat it separately. declStmt(hasDescendant(equalsNode(MovedVariable))), diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp index 0395650a82d1b..0844aa32825f6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp @@ -1815,92 +1815,129 @@ namespace GH62206 { (d) = b; // Should not warn } } // namespace GH62206 - - -std::pair<std::string, std::string> makeStringPair(std::string a, - std::string b); - -void stdTieIsReinit() { - std::string a, b; - std::move(a); - std::move(b); - std::tie(a, b) = makeStringPair("x", "y"); - a.size(); - b.size(); -} - -void stdTieWithIgnore() { - std::string a; - std::move(a); - std::tie(a, std::ignore) = makeStringPair("x", "y"); - a.size(); -} - -void stdTieIgnoreFlipped() { - std::string a; - std::move(a); - std::tie(std::ignore, a) = makeStringPair("x", "y"); - a.size(); -} - -void stdTieThreeVars() { - std::string a, b, c; - std::move(a); - std::move(b); - std::move(c); - std::tie(a, b, c) = - std::make_tuple(std::string("x"), std::string("y"), std::string("z")); - a.size(); - b.size(); - c.size(); -} - -void stdTiePartialReinit() { - std::string a, b, c; - std::move(a); - std::move(b); - std::move(c); - std::tie(b) = std::make_tuple(std::string("y")); - b.size(); - std::tie(c, b, std::ignore) = - std::make_tuple(std::string("x"), std::string("y"), std::string("z")); - b.size(); - c.size(); - a.size(); - // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved - // CHECK-NOTES: [[@LINE-11]]:3: note: move occurred here -} - -void stdTieInLoop() { - std::string a, b; - while (true) { - std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); - std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); - } -} - -template <typename T> -void stdTieReinitInTemplate() { - T a, b; - std::move(a); - std::move(b); - std::tie(a, b) = std::make_tuple(T(), T()); - a.size(); - b.size(); -} - -template <typename T> -void stdTiePartialReinitInTemplate() { - T a, b; - std::move(a); - std::tie(b) = std::make_tuple(T()); - b.size(); - a.size(); - // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved - // CHECK-NOTES: [[@LINE-5]]:3: note: move occurred here -} - -void callStdTieTemplateTests() { - stdTieReinitInTemplate<std::string>(); - stdTiePartialReinitInTemplate<std::string>(); -} + + +std::pair<std::string, std::string> makeStringPair(std::string a, + std::string b); + +void stdTieIsReinit() { + std::string a, b; + std::move(a); + std::move(b); + std::tie(a, b) = makeStringPair("x", "y"); + a.size(); + b.size(); +} + +void stdTieWithIgnore() { + std::string a; + std::move(a); + std::tie(a, std::ignore) = makeStringPair("x", "y"); + a.size(); +} + +void stdTieIgnoreFlipped() { + std::string a; + std::move(a); + std::tie(std::ignore, a) = makeStringPair("x", "y"); + a.size(); +} + +void stdTieThreeVars() { + std::string a, b, c; + std::move(a); + std::move(b); + std::move(c); + std::tie(a, b, c) = + std::make_tuple(std::string("x"), std::string("y"), std::string("z")); + a.size(); + b.size(); + c.size(); +} + +void stdTiePartialReinit() { + std::string a, b, c; + std::move(a); + std::move(b); + std::move(c); + std::tie(b) = std::make_tuple(std::string("y")); + b.size(); + std::tie(c, b, std::ignore) = + std::make_tuple(std::string("x"), std::string("y"), std::string("z")); + b.size(); + c.size(); + a.size(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-11]]:3: note: move occurred here +} + +void stdTieInLoop() { + std::string a, b; + while (true) { + std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); + std::tie(a, b) = makeStringPair(std::move(a), std::move(b)); + } +} + +template <typename T> +void stdTieReinitInTemplate() { + T a, b; + std::move(a); + std::move(b); + std::tie(a, b) = std::make_tuple(T(), T()); + a.size(); + b.size(); +} + +template <typename T> +void stdTiePartialReinitInTemplate() { + T a, b; + std::move(a); + std::tie(b) = std::make_tuple(T()); + b.size(); + a.size(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-5]]:3: note: move occurred here +} + +void callStdTieTemplateTests() { + stdTieReinitInTemplate<std::string>(); + stdTiePartialReinitInTemplate<std::string>(); +} + +void stdTieParenthesized() { + // Parenthesized std::tie on the LHS still reinitializes captured variables. + std::string a, b; + std::move(a); + std::move(b); + (std::tie(a, b)) = makeStringPair("x", "y"); // no-warning: both reinitialized + a.size(); + b.size(); +} + +void stdTieParenthesizedWithIgnore() { + // Parenthesized std::tie with std::ignore still reinitializes named variables. + std::string a; + std::move(a); + (std::tie(a, std::ignore)) = makeStringPair("x", "y"); // no-warning: a reinitialized + a.size(); +} + +void stdTieParenthesizedIgnoreFlipped() { + // std::ignore in first position inside parenthesized std::tie. + std::string a; + std::move(a); + (std::tie(std::ignore, a)) = makeStringPair("x", "y"); // no-warning: a reinitialized + a.size(); +} + +void stdTieParenthesizedPartialReinit() { + // Parenthesized std::tie only reinitializes variables named in the call. + std::string a, b; + std::move(a); + (std::tie(b)) = std::make_tuple(std::string("y")); // reinitializes b, not a + b.size(); // no-warning: b was reinitialized + a.size(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved + // CHECK-NOTES: [[@LINE-5]]:3: note: move occurred here +} >From a98c4cf9f7652429d5dc9462288cf3861f952904 Mon Sep 17 00:00:00 2001 From: Yuta Nakamura <[email protected]> Date: Mon, 1 Jun 2026 16:29:25 -0400 Subject: [PATCH 3/4] [clang-tidy] Remove obvious comment per review feedback Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 8dae9f6278459..1d92f54326659 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -138,9 +138,7 @@ makeReinitMatcher(const ValueDecl *MovedVariable, // all variables passed to std::tie because the tuple // assignment writes back through the stored references. // ignoringImplicit strips the MaterializeTemporaryExpr that - // Clang inserts when calling operator= on the prvalue tuple; - // ignoringParenImpCasts additionally handles parenthesized - // forms such as (std::tie(a, b)) = expr. + // Clang inserts when calling operator= on the prvalue tuple. binaryOperation( hasOperatorName("="), hasLHS(ignoringImplicit(ignoringParenImpCasts( >From b60bcfe92b64b8014bfbf4413c71f5f8f43b7283 Mon Sep 17 00:00:00 2001 From: Zeyi Xu <[email protected]> Date: Tue, 2 Jun 2026 09:48:20 +0800 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Zeyi Xu <[email protected]> --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 1d92f54326659..361be321185df 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -137,8 +137,6 @@ makeReinitMatcher(const ValueDecl *MovedVariable, // std::tie() assignment: std::tie(a, b) = expr reinitializes // all variables passed to std::tie because the tuple // assignment writes back through the stored references. - // ignoringImplicit strips the MaterializeTemporaryExpr that - // Clang inserts when calling operator= on the prvalue tuple. binaryOperation( hasOperatorName("="), hasLHS(ignoringImplicit(ignoringParenImpCasts( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
