[clang][dataflow] Extend flow condition in the body of a for loop

[sgatev]

Summary

Extend flow condition in the body of a for loop.

[gribozavr2]

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

3577–3580

[ymandel]

Mild preference for splitting into three tests, since it technically different code that's covered in each. But, up to you.

[sgatev]

Address comments.

[xazax.hun]

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

92

Do we support DoStmt or is that missing?

[sgatev]

Address comments.

[sgatev]

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

92

Good question. I think it's worth adding support for DoStmt, but that won't be very useful at this point because the condition will not always be true within the body of the loop. I'd be happy to follow up with a patch for it, but I'll first need to figure out what test to write. Ideas are welcome. : )

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

public:
   }
 
   void VisitWhileStmt(const WhileStmt *S) {
     auto *Cond = S->getCond();
     assert(Cond != nullptr);
     extendFlowCondition(*Cond);
   }
 
+  void VisitForStmt(const ForStmt *S) {
+    auto *Cond = S->getCond();
+    assert(Cond != nullptr);
+    extendFlowCondition(*Cond);
+  }
+
   void VisitBinaryOperator(const BinaryOperator *S) {
     assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
     auto *LHS = S->getLHS();
     assert(LHS != nullptr);
     extendFlowCondition(*LHS);
   }
 
   void VisitConditionalOperator(const ConditionalOperator *S) {

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(Code,
                      std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                      Results,
                  ASTContext &ASTCtx) {
                 ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
                 const Environment &Env = Results[0].second.Env;
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
                 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
                 ASSERT_THAT(BazDecl, NotNull());
 
                 const StorageLocation *BarLoc =
                     Env.getStorageLocation(*BarDecl, SkipPast::Reference);
                 ASSERT_THAT(BarLoc, NotNull());
 
                 const StorageLocation *BazLoc =
                     Env.getStorageLocation(*BazDecl, SkipPast::Reference);
                 EXPECT_EQ(BazLoc, BarLoc);
               });
 }
 
+TEST_F(TransferTest, IfStmtBranchExtendsFlowCondition) {
+  std::string Code = R"(
+    void target(bool Foo) {
+      if (Foo) {
+        (void)0;
+        // [[if_then]]
+      } else {
+        (void)0;
+        // [[if_else]]
+      }
+    }
+  )";
+  runDataflow(
+      Code, [](llvm::ArrayRef<
+                   std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                   Results,
+               ASTContext &ASTCtx) {
+        ASSERT_THAT(Results,
+                    ElementsAre(Pair("if_else", _), Pair("if_then", _)));
+        const Environment &ThenEnv = Results[1].second.Env;
+        const Environment &ElseEnv = Results[0].second.Env;
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+
+        BoolValue &ThenFooVal =
+            *cast<BoolValue>(ThenEnv.getValue(*FooDecl, SkipPast::None));
+        EXPECT_TRUE(ThenEnv.flowConditionImplies(ThenFooVal));
+
+        BoolValue &ElseFooVal =
+            *cast<BoolValue>(ElseEnv.getValue(*FooDecl, SkipPast::None));
+        EXPECT_TRUE(ElseEnv.flowConditionImplies(ElseEnv.makeNot(ElseFooVal)));
+      });
+}
+
+TEST_F(TransferTest, WhileStmtBranchExtendsFlowCondition) {
+  std::string Code = R"(
+    void target(bool Foo) {
+      while (Foo) {
+        (void)0;
+        // [[while_branch]]
+      }
+    }
+  )";
+  runDataflow(Code,
+              [](llvm::ArrayRef<
+                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                     Results,
+                 ASTContext &ASTCtx) {
+                ASSERT_THAT(Results, ElementsAre(Pair("while_branch", _)));
+                const Environment &Env = Results[0].second.Env;
+
+                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                BoolValue &FooVal =
+                    *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+                EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+              });
+}
+
+TEST_F(TransferTest, ForStmtBranchExtendsFlowCondition) {
+  std::string Code = R"(
+    void target(bool Foo) {
+      for (; Foo;) {
+        (void)0;
+        // [[for_branch]]
+      }
+    }
+  )";
+  runDataflow(Code,
+              [](llvm::ArrayRef<
+                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                     Results,
+                 ASTContext &ASTCtx) {
+                ASSERT_THAT(Results, ElementsAre(Pair("for_branch", _)));
+                const Environment &Env = Results[0].second.Env;
+
+                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                BoolValue &FooVal =
+                    *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
+                EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+              });
+}
+
 } // namespace