diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -189,12 +189,13 @@ } 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 @@ 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 @@ // 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(B->getBinding()); - if (ME == nullptr) - continue; - - auto *DE = dyn_cast_or_null(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(B->getBinding())) { + auto *DE = dyn_cast_or_null(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 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -346,14 +346,12 @@ TypeErasedDataflowAnalysisState &State, AnalysisContext &AC) { switch (Elt.getKind()) { - case CFGElement::Statement: { + case CFGElement::Statement: builtinTransferStatement(Elt.castAs(), State, AC); break; - } - case CFGElement::Initializer: { + case CFGElement::Initializer: builtinTransferInitializer(Elt.castAs(), 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 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3613,6 +3613,172 @@ }); } +TEST(TransferTest, StructuredBindingAssignFromTupleLikeType) { + std::string Code = R"( + namespace std { + using size_t = int; + template struct tuple_size; + template struct tuple_element; + template class tuple; + + namespace { + template + struct size_helper { static const T value = v; }; + } // namespace + + template + struct tuple_size> : size_helper {}; + + template + struct tuple_element> { + using type = __type_pack_element; + }; + + template class tuple {}; + + template + typename tuple_element>::type get(tuple); + } // namespace std + + std::tuple 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> &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(BoundFooValue)); + + const Value *BoundBarValue = + Env1.getValue(*BoundBarDecl, SkipPast::Reference); + ASSERT_THAT(BoundBarValue, NotNull()); + EXPECT_TRUE(isa(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 struct tuple_size; + template struct tuple_element; + template class tuple; + + namespace { + template + struct size_helper { static const T value = v; }; + } // namespace + + template + struct tuple_size> : size_helper {}; + + template + struct tuple_element> { + using type = __type_pack_element; + }; + + template class tuple {}; + + template + typename tuple_element>::type get(tuple); + } // namespace std + + std::tuple &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> &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(BoundFooValue)); + + const Value *BoundBarValue = + Env1.getValue(*BoundBarDecl, SkipPast::Reference); + ASSERT_THAT(BoundBarValue, NotNull()); + EXPECT_TRUE(isa(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 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -2860,6 +2860,59 @@ )"); } +TEST_P(UncheckedOptionalAccessTest, StructuredBindingsFromStruct) { + ExpectDiagnosticsFor(R"( + #include "unchecked_optional_access_test.h" + + struct kv { $ns::$optional opt; int x; }; + int target() { + auto [contents, x] = Make(); + return contents ? *contents : x; + } + )"); + + ExpectDiagnosticsFor(R"( + #include "unchecked_optional_access_test.h" + + template + struct pair { T1 fst; T2 snd; }; + int target() { + auto [contents, x] = Make, int>>(); + return contents ? *contents : x; + } + )"); +} + +TEST_P(UncheckedOptionalAccessTest, StructuredBindingsFromTupleLikeType) { + ExpectDiagnosticsFor(R"( + #include "unchecked_optional_access_test.h" + + namespace std { + template struct tuple_size; + template struct tuple_element; + template class tuple; + + template + struct tuple_size> : integral_constant {}; + + template + struct tuple_element> { + using type = __type_pack_element; + }; + + template class tuple {}; + template + typename tuple_element>::type get(tuple); + } // namespace std + + std::tuple<$ns::$optional, int> get_opt(); + void target() { + auto [content, ck] = get_opt(); + content ? *content : ""; + } + )"); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move)