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 @@ -44,6 +44,104 @@ return Env.makeAtomicBoolValue(); } +/// Evaluates sub-expressions of `BindingDecl`s in `DecompositionDecl`, i.e. +/// structured bindings. Currently these expressions are not represented in the +/// CFG and hence not evaluated as part of the regular `transfer`. This class +/// matches one of the following patterns: +/// +/// * Array binding: +/// \code +/// int a[2] = {1, 2}; +/// auto [x, y] = a; +/// \endcode +/// +/// `[x, y]` is the `DecompositionDecl` and the two `BindingDecl`s for `x` and +/// `y` bind to `ArraySubScriptExpr`s whose bases are `DeclRefExpr` that refer +/// to the `DecompositionDecl`. +/// +/// * Tuple-like type binding: +/// \code +/// std::tuple tpl(a, std::move(b)); +/// const auto& [x, y] = tpl; +/// \endcode +/// +/// `[a, b]` is the `DecompositionDecl` and the two `BindingDecl`s for `x` and +/// `y` bind to `DeclRefExpr`s that refer to `VarDecl`s of the tuple (for `a`, +/// `b`) obtained via `get()`. +/// +/// * Data members binding: +/// \code +/// struct S { +/// int a; +/// double b; +/// }; +/// S f() { return S(1, 2.3}; } +/// ... +/// const auto [x, y] = f(); +/// \endcode +/// +/// `[x, y]` is the `DecompositionDecl` and the two `BindingDecl`s for `x` and +/// `y` bind to `MemberExpr`s whose bases are `DeclRefExpr`s that refer to the +/// `DecompositionDecl`. +/// +/// FIXME: Consider adding support for structured bindings to the CFG builder. +class DecompositionVisitor : public ConstStmtVisitor { +public: + explicit DecompositionVisitor(Environment &Env) : Env(Env) {} + + void VisitDeclRefExpr(const DeclRefExpr *E) { + assert(E->getDecl() != nullptr); + auto *DeclLoc = Env.getStorageLocation(*E->getDecl(), SkipPast::None); + if (DeclLoc == nullptr) + return; + + if (DeclLoc->getType()->isReferenceType()) { + Env.setStorageLocation(*E, *DeclLoc); + } else { + auto &Loc = Env.createStorageLocation(*E); + Env.setStorageLocation(*E, Loc); + Env.setValue( + Loc, Env.takeOwnership(std::make_unique(*DeclLoc))); + } + } + + void VisitMemberExpr(const MemberExpr *E) { + ValueDecl *Member = E->getMemberDecl(); + assert(Member != nullptr); + assert(!Member->isFunctionOrFunctionTemplate()); + assert(isa(E->getBase()) && + "A member's base is expected to be DeclRefExpr"); + + // E's base hasn't been visited because it is not included in the statements + // of the CFG basic block. + assert(E->getBase() != nullptr); + Visit(E->getBase()); + + // The receiver can be either a value or a pointer to a value. Skip past the + // indirection to handle both cases. + auto *BaseLoc = cast_or_null( + Env.getStorageLocation(*E->getBase(), SkipPast::ReferenceThenPointer)); + if (BaseLoc == nullptr) + return; + + auto &MemberLoc = BaseLoc->getChild(*Member); + if (MemberLoc.getType()->isReferenceType()) { + Env.setStorageLocation(*E, MemberLoc); + } else { + auto &Loc = Env.createStorageLocation(*E); + Env.setStorageLocation(*E, Loc); + Env.setValue( + Loc, Env.takeOwnership(std::make_unique(MemberLoc))); + } + } + + // FIXME: Add support for ArraySubscriptExpr. + // FIXME: Add support for ImplicitCastExpr. + +private: + Environment &Env; +}; + class TransferVisitor : public ConstStmtVisitor { public: TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env) @@ -136,9 +234,6 @@ return; } - InitExpr = D.getInit(); - assert(InitExpr != nullptr); - if (D.getType()->isReferenceType()) { // Initializing a reference variable - do not create a reference to // reference. @@ -147,25 +242,40 @@ auto &Val = Env.takeOwnership(std::make_unique(*InitExprLoc)); Env.setValue(Loc, Val); - return; } } else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { Env.setValue(Loc, *InitExprVal); - return; } - // We arrive here in (the few) cases where an expression is intentionally - // "uninterpreted". There are two ways to handle this situation: propagate - // the status, so that uninterpreted initializers result in uninterpreted - // variables, or provide a default value. We choose the latter so that later - // refinements of the variable can be used for reasoning about the - // surrounding code. - // - // FIXME. If and when we interpret all language cases, change this to assert - // that `InitExpr` is interpreted, rather than supplying a default value - // (assuming we don't update the environment API to return references). - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); + if (Env.getValue(Loc) == nullptr) { + // We arrive here in (the few) cases where an expression is intentionally + // "uninterpreted". There are two ways to handle this situation: propagate + // the status, so that uninterpreted initializers result in uninterpreted + // variables, or provide a default value. We choose the latter so that + // later refinements of the variable can be used for reasoning about the + // surrounding code. + // + // FIXME. If and when we interpret all language cases, change this to + // assert that `InitExpr` is interpreted, rather than supplying a default + // value (assuming we don't update the environment API to return + // references). + if (Value *Val = Env.createValue(D.getType())) + Env.setValue(Loc, *Val); + } + + if (const auto *Decomp = dyn_cast(&D)) { + // If VarDecl is a DecompositionDecl, evaluate each of its bindings. This + // needs to be evaluated after initializing the values in the storage for + // `VarDecl`, as the bindings refer to them. + DecompositionVisitor Visitor(Env); + for (const auto *B : Decomp->bindings()) { + if (auto *Expr = B->getBinding()) { + Visitor.Visit(Expr); + if (auto *Loc = Env.getStorageLocation(*Expr, SkipPast::Reference)) + Env.setStorageLocation(*B, *Loc); + } + } + } } void VisitImplicitCastExpr(const ImplicitCastExpr *S) { 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 @@ -3369,4 +3369,192 @@ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator="); } +TEST_F(TransferTest, StructuredBindingAssignFromStructIntMembersToRefs) { + std::string Code = R"( + struct A { + int Foo; + int Bar; + }; + + void target() { + int Qux; + A Baz; + Baz.Foo = Qux; + auto &FooRef = Baz.Foo; + auto &BarRef = Baz.Bar; + auto &[BoundFooRef, BoundBarRef] = Baz; + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooRefDecl = findValueDecl(ASTCtx, "FooRef"); + ASSERT_THAT(FooRefDecl, NotNull()); + + const ValueDecl *BarRefDecl = findValueDecl(ASTCtx, "BarRef"); + ASSERT_THAT(BarRefDecl, NotNull()); + + const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux"); + ASSERT_THAT(QuxDecl, NotNull()); + + const ValueDecl *BoundFooRefDecl = findValueDecl(ASTCtx, "BoundFooRef"); + ASSERT_THAT(BoundFooRefDecl, NotNull()); + + const ValueDecl *BoundBarRefDecl = findValueDecl(ASTCtx, "BoundBarRef"); + ASSERT_THAT(BoundBarRefDecl, NotNull()); + + const StorageLocation *FooRefLoc = + Env.getStorageLocation(*FooRefDecl, SkipPast::Reference); + ASSERT_THAT(FooRefLoc, NotNull()); + + const StorageLocation *BarRefLoc = + Env.getStorageLocation(*BarRefDecl, SkipPast::Reference); + ASSERT_THAT(BarRefLoc, NotNull()); + + const Value *QuxVal = Env.getValue(*QuxDecl, SkipPast::None); + ASSERT_THAT(QuxVal, NotNull()); + + const StorageLocation *BoundFooRefLoc = + Env.getStorageLocation(*BoundFooRefDecl, SkipPast::Reference); + EXPECT_EQ(BoundFooRefLoc, FooRefLoc); + + const StorageLocation *BoundBarRefLoc = + Env.getStorageLocation(*BoundBarRefDecl, SkipPast::Reference); + EXPECT_EQ(BoundBarRefLoc, BarRefLoc); + + EXPECT_EQ(Env.getValue(*BoundFooRefDecl, SkipPast::Reference), QuxVal); + }); +} + +TEST_F(TransferTest, StructuredBindingAssignFromStructRefMembersToRefs) { + std::string Code = R"( + struct A { + int &Foo; + int &Bar; + }; + + void target(A Baz) { + int Qux; + Baz.Foo = Qux; + auto &FooRef = Baz.Foo; + auto &BarRef = Baz.Bar; + auto &[BoundFooRef, BoundBarRef] = Baz; + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooRefDecl = findValueDecl(ASTCtx, "FooRef"); + ASSERT_THAT(FooRefDecl, NotNull()); + + const ValueDecl *BarRefDecl = findValueDecl(ASTCtx, "BarRef"); + ASSERT_THAT(BarRefDecl, NotNull()); + + const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux"); + ASSERT_THAT(QuxDecl, NotNull()); + + const ValueDecl *BoundFooRefDecl = findValueDecl(ASTCtx, "BoundFooRef"); + ASSERT_THAT(BoundFooRefDecl, NotNull()); + + const ValueDecl *BoundBarRefDecl = findValueDecl(ASTCtx, "BoundBarRef"); + ASSERT_THAT(BoundBarRefDecl, NotNull()); + + const StorageLocation *FooRefLoc = + Env.getStorageLocation(*FooRefDecl, SkipPast::Reference); + ASSERT_THAT(FooRefLoc, NotNull()); + + const StorageLocation *BarRefLoc = + Env.getStorageLocation(*BarRefDecl, SkipPast::Reference); + ASSERT_THAT(BarRefLoc, NotNull()); + + const Value *QuxVal = Env.getValue(*QuxDecl, SkipPast::None); + ASSERT_THAT(QuxVal, NotNull()); + + const StorageLocation *BoundFooRefLoc = + Env.getStorageLocation(*BoundFooRefDecl, SkipPast::Reference); + EXPECT_EQ(BoundFooRefLoc, FooRefLoc); + + const StorageLocation *BoundBarRefLoc = + Env.getStorageLocation(*BoundBarRefDecl, SkipPast::Reference); + EXPECT_EQ(BoundBarRefLoc, BarRefLoc); + + EXPECT_EQ(Env.getValue(*BoundFooRefDecl, SkipPast::Reference), QuxVal); + }); +} + +TEST_F(TransferTest, StructuredBindingAssignFromStructIntMembersToInts) { + std::string Code = R"( + struct A { + int Foo; + int Bar; + }; + + void target() { + int Qux; + A Baz; + Baz.Foo = Qux; + auto &FooRef = Baz.Foo; + auto &BarRef = Baz.Bar; + auto [BoundFoo, BoundBar] = Baz; + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooRefDecl = findValueDecl(ASTCtx, "FooRef"); + ASSERT_THAT(FooRefDecl, NotNull()); + + const ValueDecl *BarRefDecl = findValueDecl(ASTCtx, "BarRef"); + ASSERT_THAT(BarRefDecl, NotNull()); + + const ValueDecl *BoundFooDecl = findValueDecl(ASTCtx, "BoundFoo"); + ASSERT_THAT(BoundFooDecl, NotNull()); + + const ValueDecl *BoundBarDecl = findValueDecl(ASTCtx, "BoundBar"); + ASSERT_THAT(BoundBarDecl, NotNull()); + + const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux"); + ASSERT_THAT(QuxDecl, NotNull()); + + const StorageLocation *FooRefLoc = + Env.getStorageLocation(*FooRefDecl, SkipPast::Reference); + ASSERT_THAT(FooRefLoc, NotNull()); + + const StorageLocation *BarRefLoc = + Env.getStorageLocation(*BarRefDecl, SkipPast::Reference); + ASSERT_THAT(BarRefLoc, NotNull()); + + const Value *QuxVal = Env.getValue(*QuxDecl, SkipPast::None); + ASSERT_THAT(QuxVal, NotNull()); + + const StorageLocation *BoundFooLoc = + Env.getStorageLocation(*BoundFooDecl, SkipPast::Reference); + EXPECT_NE(BoundFooLoc, FooRefLoc); + + const StorageLocation *BoundBarLoc = + Env.getStorageLocation(*BoundBarDecl, SkipPast::Reference); + EXPECT_NE(BoundBarLoc, BarRefLoc); + + EXPECT_EQ(Env.getValue(*BoundFooDecl, SkipPast::Reference), QuxVal); + }); +} + } // namespace