sgatev updated this revision to Diff 432728.
sgatev added a comment.

Rebase main.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120495/new/

https://reviews.llvm.org/D120495

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ 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<std::string, DataflowAnalysisState<NoopLattice>>>
+                   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<std::string, DataflowAnalysisState<NoopLattice>>>
+                   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<std::string, DataflowAnalysisState<NoopLattice>>>
+                   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
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ 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<float&, char&&> 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<i>()`.
+///
+/// * 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<DecompositionVisitor> {
+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<ReferenceValue>(*DeclLoc)));
+    }
+  }
+
+  void VisitMemberExpr(const MemberExpr *E) {
+    ValueDecl *Member = E->getMemberDecl();
+    assert(Member != nullptr);
+    assert(!Member->isFunctionOrFunctionTemplate());
+    assert(isa<DeclRefExpr>(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<AggregateStorageLocation>(
+        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<ReferenceValue>(MemberLoc)));
+    }
+  }
+
+  // FIXME: Add support for ArraySubscriptExpr.
+  // FIXME: Add support for ImplicitCastExpr.
+
+private:
+  Environment &Env;
+};
+
 class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
 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<ReferenceValue>(*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<DecompositionDecl>(&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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to