[clang][dataflow] Add transfer functions for bind temporary and static cast

[sgatev]

Summary

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

[xazax.hun]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

90–120

Wouldn't you dereference a null pointer for group decls?

[sgatev]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

90–120

I couldn't figure what code to write that would create a group decl. Maybe because of https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/CFG.cpp#L2842-L2856?

[xazax.hun]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

90–120

Oh, interesting! Let's add a comment about this.

[sgatev]

Address reviewers' comments.

clang/lib/Analysis/FlowSensitive/Transfer.cpp

if (S->getDecl()->getType()->isReferenceType()) {
       auto &Loc = Env.createStorageLocation(*S);
       auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*DeclLoc));
       Env.setStorageLocation(*S, Loc);
       Env.setValue(Loc, Val);
     }
   }
 
   void VisitDeclStmt(const DeclStmt *S) {
-    // FIXME: Add support for group decls, e.g: `int a, b;`
-    if (S->isSingleDecl()) {
-      if (const auto *D = dyn_cast<VarDecl>(S->getSingleDecl())) {
-        visitVarDecl(*D);
-      }
+    // Group decls are converted into single decls in the CFG so the cast below
+    // is safe.
+    const auto &D = *cast<VarDecl>(S->getSingleDecl());
+    auto &Loc = Env.createStorageLocation(D);
+    Env.setStorageLocation(D, Loc);
+
+    const Expr *InitExpr = D.getInit();
+    if (InitExpr == nullptr) {
+      // No initializer expression - associate `Loc` with a new value.
+      Env.initValueInStorageLocation(Loc, D.getType());
+      return;
+
+    // The CFG does not contain `ParenExpr` as top-level statements in basic
+    // blocks, however sub-expressions can still be of that type.
+    InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+    assert(InitExpr != nullptr);
+
+    if (D.getType()->isReferenceType()) {
+      // Initializing a reference variable - do not create a reference to
+      // reference.
+      if (auto *InitExprLoc =
+              Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
+        auto &Val =
+            Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
+        Env.setValue(Loc, Val);
+      } else {
+        // FIXME: The initializer expression must always be assigned a value.
+        // Replace this with an assert when we have sufficient coverage of
+        // language features.
+        Env.initValueInStorageLocation(Loc, D.getType());
+      }
+      return;
+    }
+
+    if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
+      Env.setValue(Loc, *InitExprVal);
+    } else if (!D.getType()->isStructureOrClassType()) {
+      // FIXME: The initializer expression must always be assigned a value.
+      // Replace this with an assert when we have sufficient coverage of
+      // language features.
+      Env.initValueInStorageLocation(Loc, D.getType());
+    } else {
+      llvm_unreachable("structs and classes must always be assigned values");
     }
   }
 
   void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
     // The CFG does not contain `ParenExpr` as top-level statements in basic
     // blocks, however sub-expressions can still be of that type.
     assert(S->getSubExpr() != nullptr);
     const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *S) {
 
     auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
     if (SubExprLoc == nullptr)
       return;
 
     Env.setStorageLocation(*S, *SubExprLoc);
   }
 
-  // FIXME: Add support for:
-  // - CXXBindTemporaryExpr
-  // - CXXBoolLiteralExpr
-  // - CXXStaticCastExpr
-
-private:
-  void visitVarDecl(const VarDecl &D) {
-    auto &Loc = Env.createStorageLocation(D);
-    Env.setStorageLocation(D, Loc);
+  void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
+    const Expr *SubExpr = S->getSubExpr();
+    assert(SubExpr != nullptr);
 
-    const Expr *InitExpr = D.getInit();
-    if (InitExpr == nullptr) {
-      // No initializer expression - associate `Loc` with a new value.
-      Env.initValueInStorageLocation(Loc, D.getType());
+    auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+    if (SubExprLoc == nullptr)
       return;
-    }
+
+    Env.setStorageLocation(*S, *SubExprLoc);
 
-    // The CFG does not contain `ParenExpr` as top-level statements in basic
-    // blocks, however sub-expressions can still be of that type.
-    InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
-    assert(InitExpr != nullptr);
+  void VisitCXXStaticCastExpr(const CXXStaticCastExpr *S) {
+    if (S->getCastKind() == CK_NoOp) {
+      const Expr *SubExpr = S->getSubExpr();
+      assert(SubExpr != nullptr);
 
-    if (D.getType()->isReferenceType()) {
-      // Initializing a reference variable - do not create a reference to
-      // reference.
-      if (auto *InitExprLoc =
-              Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
-        auto &Val =
-            Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
-        Env.setValue(Loc, Val);
-      } else {
-        // FIXME: The initializer expression must always be assigned a value.
-        // Replace this with an assert when we have sufficient coverage of
-        // language features.
-        Env.initValueInStorageLocation(Loc, D.getType());
-      }
+      auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+      if (SubExprLoc == nullptr)
         return;
-    }
 
-    if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
-      Env.setValue(Loc, *InitExprVal);
-    } else if (!D.getType()->isStructureOrClassType()) {
-      // FIXME: The initializer expression must always be assigned a value.
-      // Replace this with an assert when we have sufficient coverage of
-      // language features.
-      Env.initValueInStorageLocation(Loc, D.getType());
-    } else {
-      llvm_unreachable("structs and classes must always be assigned values");
+      Env.setStorageLocation(*S, *SubExprLoc);
     }
   }
 
+  // FIXME: Add support for:
+  // - CXXBoolLiteralExpr
+
+private:
   Environment &Env;
 };
 
 void transfer(const Stmt &S, Environment &Env) {
   assert(!isa<ParenExpr>(&S));
   TransferVisitor(Env).Visit(&S);
 }
 
 } // namespace dataflow
 } // namespace clang

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(
 
         const auto *BazPtrVal =
             cast<PointerValue>(&BarPointeeVal->getChild(*BazPtrDecl));
         const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc();
         EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
       });
 }
 
+TEST_F(TransferTest, MultipleVarsDecl) {
+  std::string Code = R"(
+    void target() {
+      int Foo, Bar;
+      (void)0;
+      // [[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 *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+                ASSERT_THAT(BarDecl, NotNull());
+
+                const StorageLocation *FooLoc =
+                    Env.getStorageLocation(*FooDecl, SkipPast::None);
+                ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
+
+                const StorageLocation *BarLoc =
+                    Env.getStorageLocation(*BarDecl, SkipPast::None);
+                ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
+
+                const Value *FooVal = Env.getValue(*FooLoc);
+                EXPECT_TRUE(isa_and_nonnull<IntegerValue>(FooVal));
+
+                const Value *BarVal = Env.getValue(*BarLoc);
+                EXPECT_TRUE(isa_and_nonnull<IntegerValue>(BarVal));
+              });
+}
+
 TEST_F(TransferTest, JoinVarDecl) {
   std::string Code = R"(
     void target(bool B) {
       int Foo;
       // [[p1]]
       if (B) {
         int Bar;
         // [[p2]]
runDataflow(
         EXPECT_EQ(FooVal2, BarVal1);
 
         const auto *FooBazVal2 =
             cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl)));
         EXPECT_EQ(FooBazVal2, BarBazVal1);
       });
 }
 
+TEST_F(TransferTest, BindTemporary) {
+  std::string Code = R"(
+    struct A {
+      virtual ~A() = default;
+
+      int Baz;
+    };
+
+    void target(A Foo) {
+      int Bar = A(Foo).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 *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+                ASSERT_THAT(BarDecl, NotNull());
+
+                const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+                ASSERT_THAT(BazDecl, NotNull());
+
+                const auto &FooVal =
+                    *cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None));
+                const auto *BarVal =
+                    cast<IntegerValue>(Env.getValue(*BarDecl, SkipPast::None));
+                EXPECT_EQ(BarVal, &FooVal.getChild(*BazDecl));
+              });
+}
+
+TEST_F(TransferTest, StaticCast) {
+  std::string Code = R"(
+    void target(int Foo) {
+      int Bar = static_cast<int>(Foo);
+      // [[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 *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+                ASSERT_THAT(BarDecl, NotNull());
+
+                const auto *FooVal =
+                    cast<IntegerValue>(Env.getValue(*FooDecl, SkipPast::None));
+                const auto *BarVal =
+                    cast<IntegerValue>(Env.getValue(*BarDecl, SkipPast::None));
+                EXPECT_EQ(FooVal, BarVal);
+              });
+}
+
 } // namespace