[clang] ef46354 - [clang][dataflow] Add support for structured bindings of tuple-like types.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 9 10:58:06 PST 2022
Author: Yitzhak Mandelbaum
Date: 2022-12-09T18:58:00Z
New Revision: ef4635452f3a23ee29f7586148e19e8c84471a59
URL: https://github.com/llvm/llvm-project/commit/ef4635452f3a23ee29f7586148e19e8c84471a59
DIFF: https://github.com/llvm/llvm-project/commit/ef4635452f3a23ee29f7586148e19e8c84471a59.diff
LOG: [clang][dataflow] Add support for structured bindings of tuple-like types.
This patch adds interpretation of binding declarations resulting from a
structured binding (`DecompositionDecl`) to a tuple-like type. Currently, the
framework only supports binding to a struct.
Fixes issue #57252.
Differential Revision: https://reviews.llvm.org/D139544
Added:
Modified:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 55510f486163c..43d004697799c 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -189,12 +189,13 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
void VisitDeclRefExpr(const DeclRefExpr *S) {
- assert(S->getDecl() != nullptr);
- auto *DeclLoc = Env.getStorageLocation(*S->getDecl(), SkipPast::None);
+ const ValueDecl *VD = S->getDecl();
+ assert(VD != nullptr);
+ auto *DeclLoc = Env.getStorageLocation(*VD, SkipPast::None);
if (DeclLoc == nullptr)
return;
- if (S->getDecl()->getType()->isReferenceType()) {
+ if (VD->getType()->isReferenceType()) {
Env.setStorageLocation(*S, *DeclLoc);
} else {
auto &Loc = Env.createStorageLocation(*S);
@@ -213,8 +214,15 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (D.hasGlobalStorage())
return;
- auto &Loc = Env.createStorageLocation(D);
- Env.setStorageLocation(D, Loc);
+ // The storage location for `D` could have been created earlier, before the
+ // variable's declaration statement (for example, in the case of
+ // BindingDecls).
+ auto *MaybeLoc = Env.getStorageLocation(D, SkipPast::None);
+ if (MaybeLoc == nullptr) {
+ MaybeLoc = &Env.createStorageLocation(D);
+ Env.setStorageLocation(D, *MaybeLoc);
+ }
+ auto &Loc = *MaybeLoc;
const Expr *InitExpr = D.getInit();
if (InitExpr == nullptr) {
@@ -258,24 +266,30 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// needs to be evaluated after initializing the values in the storage for
// VarDecl, as the bindings refer to them.
// FIXME: Add support for ArraySubscriptExpr.
- // FIXME: Consider adding AST nodes that are used for structured bindings
- // to the CFG.
+ // FIXME: Consider adding AST nodes used in BindingDecls to the CFG.
for (const auto *B : Decomp->bindings()) {
- auto *ME = dyn_cast_or_null<MemberExpr>(B->getBinding());
- if (ME == nullptr)
- continue;
-
- auto *DE = dyn_cast_or_null<DeclRefExpr>(ME->getBase());
- if (DE == nullptr)
- continue;
-
- // ME and its base haven't been visited because they aren't included in
- // the statements of the CFG basic block.
- VisitDeclRefExpr(DE);
- VisitMemberExpr(ME);
-
- if (auto *Loc = Env.getStorageLocation(*ME, SkipPast::Reference))
- Env.setStorageLocation(*B, *Loc);
+ if (auto *ME = dyn_cast_or_null<MemberExpr>(B->getBinding())) {
+ auto *DE = dyn_cast_or_null<DeclRefExpr>(ME->getBase());
+ if (DE == nullptr)
+ continue;
+
+ // ME and its base haven't been visited because they aren't included
+ // in the statements of the CFG basic block.
+ VisitDeclRefExpr(DE);
+ VisitMemberExpr(ME);
+
+ if (auto *Loc = Env.getStorageLocation(*ME, SkipPast::Reference))
+ Env.setStorageLocation(*B, *Loc);
+ } else if (auto *VD = B->getHoldingVar()) {
+ // Holding vars are used to back the BindingDecls of tuple-like
+ // types. The holding var declarations appear *after* this statement,
+ // so we have to create a location or them here to share with `B`. We
+ // don't visit the binding, because we know it will be a DeclRefExpr
+ // to `VD`.
+ auto &VDLoc = Env.createStorageLocation(*VD);
+ Env.setStorageLocation(*VD, VDLoc);
+ Env.setStorageLocation(*B, VDLoc);
+ }
}
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 9576287ea6740..9c6a3ba679347 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -346,14 +346,12 @@ void builtinTransfer(const CFGElement &Elt,
TypeErasedDataflowAnalysisState &State,
AnalysisContext &AC) {
switch (Elt.getKind()) {
- case CFGElement::Statement: {
+ case CFGElement::Statement:
builtinTransferStatement(Elt.castAs<CFGStmt>(), State, AC);
break;
- }
- case CFGElement::Initializer: {
+ case CFGElement::Initializer:
builtinTransferInitializer(Elt.castAs<CFGInitializer>(), State);
break;
- }
default:
// FIXME: Evaluate other kinds of `CFGElement`.
break;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 7e7be3c9e07ef..299837aee9289 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3613,6 +3613,172 @@ TEST(TransferTest, StructuredBindingAssignFromStructIntMembersToInts) {
});
}
+TEST(TransferTest, StructuredBindingAssignFromTupleLikeType) {
+ std::string Code = R"(
+ namespace std {
+ using size_t = int;
+ template <class> struct tuple_size;
+ template <std::size_t, class> struct tuple_element;
+ template <class...> class tuple;
+
+ namespace {
+ template <class T, T v>
+ struct size_helper { static const T value = v; };
+ } // namespace
+
+ template <class... T>
+ struct tuple_size<tuple<T...>> : size_helper<std::size_t, sizeof...(T)> {};
+
+ template <std::size_t I, class... T>
+ struct tuple_element<I, tuple<T...>> {
+ using type = __type_pack_element<I, T...>;
+ };
+
+ template <class...> class tuple {};
+
+ template <std::size_t I, class... T>
+ typename tuple_element<I, tuple<T...>>::type get(tuple<T...>);
+ } // namespace std
+
+ std::tuple<bool, int> makeTuple();
+
+ void target(bool B) {
+ auto [BoundFoo, BoundBar] = makeTuple();
+ bool Baz;
+ // Include if-then-else to test interaction of `BindingDecl` with join.
+ if (B) {
+ Baz = BoundFoo;
+ (void)BoundBar;
+ // [[p1]]
+ } else {
+ Baz = BoundFoo;
+ }
+ (void)0;
+ // [[p2]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
+ const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
+
+ const ValueDecl *BoundFooDecl = findValueDecl(ASTCtx, "BoundFoo");
+ ASSERT_THAT(BoundFooDecl, NotNull());
+
+ const ValueDecl *BoundBarDecl = findValueDecl(ASTCtx, "BoundBar");
+ ASSERT_THAT(BoundBarDecl, NotNull());
+
+ const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
+
+ const Value *BoundFooValue =
+ Env1.getValue(*BoundFooDecl, SkipPast::Reference);
+ ASSERT_THAT(BoundFooValue, NotNull());
+ EXPECT_TRUE(isa<BoolValue>(BoundFooValue));
+
+ const Value *BoundBarValue =
+ Env1.getValue(*BoundBarDecl, SkipPast::Reference);
+ ASSERT_THAT(BoundBarValue, NotNull());
+ EXPECT_TRUE(isa<IntegerValue>(BoundBarValue));
+
+ // Test that a `DeclRefExpr` to a `BindingDecl` works as expected.
+ EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+
+ const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
+
+ // Test that `BoundFooDecl` retains the value we expect, after the join.
+ BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
+ EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+ });
+}
+
+TEST(TransferTest, StructuredBindingAssignRefFromTupleLikeType) {
+ std::string Code = R"(
+ namespace std {
+ using size_t = int;
+ template <class> struct tuple_size;
+ template <std::size_t, class> struct tuple_element;
+ template <class...> class tuple;
+
+ namespace {
+ template <class T, T v>
+ struct size_helper { static const T value = v; };
+ } // namespace
+
+ template <class... T>
+ struct tuple_size<tuple<T...>> : size_helper<std::size_t, sizeof...(T)> {};
+
+ template <std::size_t I, class... T>
+ struct tuple_element<I, tuple<T...>> {
+ using type = __type_pack_element<I, T...>;
+ };
+
+ template <class...> class tuple {};
+
+ template <std::size_t I, class... T>
+ typename tuple_element<I, tuple<T...>>::type get(tuple<T...>);
+ } // namespace std
+
+ std::tuple<bool, int> &getTuple();
+
+ void target(bool B) {
+ auto &[BoundFoo, BoundBar] = getTuple();
+ bool Baz;
+ // Include if-then-else to test interaction of `BindingDecl` with join.
+ if (B) {
+ Baz = BoundFoo;
+ (void)BoundBar;
+ // [[p1]]
+ } else {
+ Baz = BoundFoo;
+ }
+ (void)0;
+ // [[p2]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
+ const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
+
+ const ValueDecl *BoundFooDecl = findValueDecl(ASTCtx, "BoundFoo");
+ ASSERT_THAT(BoundFooDecl, NotNull());
+
+ const ValueDecl *BoundBarDecl = findValueDecl(ASTCtx, "BoundBar");
+ ASSERT_THAT(BoundBarDecl, NotNull());
+
+ const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
+
+ const Value *BoundFooValue =
+ Env1.getValue(*BoundFooDecl, SkipPast::Reference);
+ ASSERT_THAT(BoundFooValue, NotNull());
+ EXPECT_TRUE(isa<BoolValue>(BoundFooValue));
+
+ const Value *BoundBarValue =
+ Env1.getValue(*BoundBarDecl, SkipPast::Reference);
+ ASSERT_THAT(BoundBarValue, NotNull());
+ EXPECT_TRUE(isa<IntegerValue>(BoundBarValue));
+
+ // Test that a `DeclRefExpr` to a `BindingDecl` (with reference type)
+ // works as expected. We don't test aliasing properties of the
+ // reference, because we don't model `std::get` and so have no way to
+ // equate separate references into the tuple.
+ EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+
+ const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
+
+ // Test that `BoundFooDecl` retains the value we expect, after the join.
+ BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
+ EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+ });
+}
+// TODO: ref binding
+
TEST(TransferTest, BinaryOperatorComma) {
std::string Code = R"(
void target(int Foo, int Bar) {
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index b1a1282edabb6..74fb73804a1f9 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2860,6 +2860,59 @@ TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
)");
}
+TEST_P(UncheckedOptionalAccessTest, StructuredBindingsFromStruct) {
+ ExpectDiagnosticsFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ struct kv { $ns::$optional<int> opt; int x; };
+ int target() {
+ auto [contents, x] = Make<kv>();
+ return contents ? *contents : x;
+ }
+ )");
+
+ ExpectDiagnosticsFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ template <typename T1, typename T2>
+ struct pair { T1 fst; T2 snd; };
+ int target() {
+ auto [contents, x] = Make<pair<$ns::$optional<int>, int>>();
+ return contents ? *contents : x;
+ }
+ )");
+}
+
+TEST_P(UncheckedOptionalAccessTest, StructuredBindingsFromTupleLikeType) {
+ ExpectDiagnosticsFor(R"(
+ #include "unchecked_optional_access_test.h"
+
+ namespace std {
+ template <class> struct tuple_size;
+ template <size_t, class> struct tuple_element;
+ template <class...> class tuple;
+
+ template <class... T>
+ struct tuple_size<tuple<T...>> : integral_constant<size_t, sizeof...(T)> {};
+
+ template <size_t I, class... T>
+ struct tuple_element<I, tuple<T...>> {
+ using type = __type_pack_element<I, T...>;
+ };
+
+ template <class...> class tuple {};
+ template <size_t I, class... T>
+ typename tuple_element<I, tuple<T...>>::type get(tuple<T...>);
+ } // namespace std
+
+ std::tuple<$ns::$optional<const char *>, int> get_opt();
+ void target() {
+ auto [content, ck] = get_opt();
+ content ? *content : "";
+ }
+ )");
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
More information about the cfe-commits
mailing list