https://github.com/yuhaouy updated https://github.com/llvm/llvm-project/pull/227526
>From b68e0899d91c5f0995887f89dd6755f045caacb4 Mon Sep 17 00:00:00 2001 From: Yu Hao <[email protected]> Date: Wed, 30 Sep 2026 00:01:38 +0000 Subject: [PATCH 1/3] Model std::exchange. --- .../Models/UncheckedOptionalAccessModel.cpp | 48 ++++++++++ .../Analysis/FlowSensitive/MockHeaders.cpp | 3 + .../UncheckedOptionalAccessModelTest.cpp | 94 ++++++++++++++++++- 3 files changed, 144 insertions(+), 1 deletion(-) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 568564fb361f4..0b1f0855d7564 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -307,6 +307,12 @@ auto isStdSwapCall() { hasArgument(1, hasOptionalOrDerivedType())); } +auto isStdExchangeCall(ast_matchers::internal::Matcher<Expr> NewValue) { + return callExpr( + callee(functionDecl(hasName("std::exchange"))), argumentCountIs(2), + hasArgument(0, hasOptionalOrDerivedType()), hasArgument(1, NewValue)); +} + auto isStdForwardCall() { return callExpr(callee(functionDecl(hasName("std::forward"))), argumentCountIs(1), @@ -848,6 +854,42 @@ void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &, transferSwap(Arg0Loc, Arg1Loc, State.Env); } +/// Models `std::exchange(Obj, NewValue)`, which initializes its result from the +/// old value of `Obj` and then assigns `NewValue` to `Obj`. `NewHasValueVal` is +/// the "has_value" property of `Obj` after the assignment. +void transferStdExchange(const CallExpr *E, BoolValue &NewHasValueVal, + LatticeTransferState &State) { + assert(E->getNumArgs() == 2); + // `ObjLoc` is null if the first argument is not modeled. + auto *ObjLoc = State.Env.get<RecordStorageLocation>(*E->getArg(0)); + + BoolValue *OldHasValueVal = getHasValue(State.Env, ObjLoc); + if (OldHasValueVal == nullptr) + OldHasValueVal = &State.Env.makeAtomicBoolValue(); + setHasValue(State.Env.getResultObjectLocation(*E), *OldHasValueVal, + State.Env); + + if (ObjLoc != nullptr) + setHasValue(*ObjLoc, NewHasValueVal, State.Env); +} + +void transferStdExchangeNulloptCall(const CallExpr *E, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferStdExchange(E, State.Env.getBoolLiteralValue(false), State); +} + +void transferStdExchangeValueOrConversionCall( + const CallExpr *E, const MatchFinder::MatchResult &MatchRes, + LatticeTransferState &State) { + assert(E->getNumArgs() == 2); + transferStdExchange( + E, + valueOrConversionHasValue(E->getArg(0)->getType().getNonReferenceType(), + *E->getArg(1), MatchRes, State), + State); +} + void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { assert(E->getNumArgs() == 1); @@ -1111,6 +1153,12 @@ auto buildTransferMatchSwitch() { // std::swap .CaseOfCFGStmt<CallExpr>(isStdSwapCall(), transferStdSwapCall) + // std::exchange + .CaseOfCFGStmt<CallExpr>(isStdExchangeCall(hasNulloptType()), + transferStdExchangeNulloptCall) + .CaseOfCFGStmt<CallExpr>(isStdExchangeCall(unless(hasNulloptType())), + transferStdExchangeValueOrConversionCall) + // std::forward .CaseOfCFGStmt<CallExpr>(isStdForwardCall(), transferStdForwardCall) diff --git a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp index c5ad8ea4dcb1f..4d4a6163ea0d1 100644 --- a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp @@ -564,6 +564,9 @@ constexpr remove_reference_t<T>&& move(T&& x); template <typename T> void swap(T& a, T& b) noexcept; +template <typename T, typename U = T> +T exchange(T& obj, U&& new_value); + } // namespace std #endif // UTILITY_H diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index f34568a072c32..7f01069056674 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1168,6 +1168,99 @@ TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueRightUnset) { )"); } +TEST_P(UncheckedOptionalAccessTest, StdExchange) { + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = 3; + + $ns::$optional<int> opt2 = std::exchange(opt1, $ns::nullopt); + + opt1.value(); // [[unsafe]] + + opt2.value(); + } + )"); + + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = $ns::nullopt; + + $ns::$optional<int> opt2 = std::exchange(opt1, 3); + + opt1.value(); + + opt2.value(); // [[unsafe]] + } + )"); + + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Foo {}; + + struct Bar { + Bar(const Foo&); + }; + + void target() { + $ns::$optional<Foo> opt1 = Foo(); + $ns::$optional<Bar> opt2 = $ns::nullopt; + + std::exchange(opt2, opt1); + + opt2.value(); + } + )"); + + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + struct Foo {}; + + struct Bar { + Bar(const Foo&); + }; + + void target() { + $ns::$optional<Foo> opt1 = $ns::nullopt; + $ns::$optional<Bar> opt2 = Foo(); + + std::exchange(opt2, opt1); + + opt2.value(); // [[unsafe]] + } + )"); + + ExpectDiagnosticsFor( + R"( + #include "unchecked_optional_access_test.h" + + struct L { $ns::$optional<int> hd; L* tl; }; + + void target() { + $ns::$optional<int> opt1 = 3; + L bar; + + // Any `tl` beyond the first is not modeled. + std::exchange(opt1, bar.tl->tl->hd); + + opt1.value(); // [[unsafe]] + + $ns::$optional<int> opt2 = std::exchange(bar.tl->tl->hd, $ns::nullopt); + + opt2.value(); // [[unsafe]] + } + )"); +} + TEST_P(UncheckedOptionalAccessTest, UniquePtrToOptional) { // We suppress diagnostics for optionals in smart pointers (other than // `optional` itself). @@ -1269,7 +1362,6 @@ TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) { )"); } - TEST_P(UncheckedOptionalAccessTest, EqualityCheckLeftSet) { ExpectDiagnosticsFor( R"( >From d369b29fb51b06038370094016ce3bb748375104 Mon Sep 17 00:00:00 2001 From: Yu Hao <[email protected]> Date: Wed, 21 Oct 2026 23:58:01 +0000 Subject: [PATCH 2/3] Split out cases for implicit conversion and unmodeled loc to their own tests and remove extra empty lines in the tests. --- .../UncheckedOptionalAccessModelTest.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 7f01069056674..5d77eea874eb2 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1179,7 +1179,6 @@ TEST_P(UncheckedOptionalAccessTest, StdExchange) { $ns::$optional<int> opt2 = std::exchange(opt1, $ns::nullopt); opt1.value(); // [[unsafe]] - opt2.value(); } )"); @@ -1194,11 +1193,12 @@ TEST_P(UncheckedOptionalAccessTest, StdExchange) { $ns::$optional<int> opt2 = std::exchange(opt1, 3); opt1.value(); - opt2.value(); // [[unsafe]] } )"); +} +TEST_P(UncheckedOptionalAccessTest, StdExchangeImplicitConversion) { ExpectDiagnosticsFor( R"( #include "unchecked_optional_access_test.h" @@ -1238,7 +1238,9 @@ TEST_P(UncheckedOptionalAccessTest, StdExchange) { opt2.value(); // [[unsafe]] } )"); +} +TEST_P(UncheckedOptionalAccessTest, StdExchangeUnmodeledLoc) { ExpectDiagnosticsFor( R"( #include "unchecked_optional_access_test.h" @@ -1248,14 +1250,14 @@ TEST_P(UncheckedOptionalAccessTest, StdExchange) { void target() { $ns::$optional<int> opt1 = 3; L bar; + bar.hd = 4; + bar.tl = &bar; - // Any `tl` beyond the first is not modeled. + // bar.tl->tl->hd has value, but any `tl` beyond the first is not modeled. std::exchange(opt1, bar.tl->tl->hd); - opt1.value(); // [[unsafe]] $ns::$optional<int> opt2 = std::exchange(bar.tl->tl->hd, $ns::nullopt); - opt2.value(); // [[unsafe]] } )"); >From 8d6a64140ca43c26c86a538815b86e0ffb58a3aa Mon Sep 17 00:00:00 2001 From: Yu Hao <[email protected]> Date: Thu, 22 Oct 2026 00:42:46 +0000 Subject: [PATCH 3/3] Fix test. The unmodeled loc test failed because `bar.tl = &bar` makes `bar.tl->tl` become modeled. --- .../FlowSensitive/UncheckedOptionalAccessModelTest.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 5d77eea874eb2..a44da83314279 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1247,13 +1247,11 @@ TEST_P(UncheckedOptionalAccessTest, StdExchangeUnmodeledLoc) { struct L { $ns::$optional<int> hd; L* tl; }; - void target() { + void target(L bar) { $ns::$optional<int> opt1 = 3; - L bar; - bar.hd = 4; - bar.tl = &bar; + if (!bar.tl->tl->hd) return; - // bar.tl->tl->hd has value, but any `tl` beyond the first is not modeled. + // Any `tl` beyond the first is not modeled. std::exchange(opt1, bar.tl->tl->hd); opt1.value(); // [[unsafe]] _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
