[clang] [dataflow] CXXForRangeStmt should extend flow condition (PR #80989)

via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 19 01:55:16 PDT 2024


================
@@ -1710,4 +1710,25 @@ TEST_F(TopTest, ForRangeStmtConverges) {
                 // analysis converged.
               });
 }
+
+TEST_F(TopTest, ForRangeStmtHasFlowCondition) {
+  std::string Code = R"(
+    #include <array>
+    void target(bool Foo) {
+      std::array<int, 5> t;
+      for (auto& i : t) {
+        (void)0;
+        /*[[p1]]*/
+      }
+    }
+  )";
+  runDataflow(Code,
+              [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+                 const AnalysisOutputs &AO) {
+                  ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1"));
+                  const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
+                  ASSERT_TRUE(Env1.proves(Env1.arena().makeAtomRef(Env1.getFlowConditionToken())));
----------------
martinboehme wrote:

```suggestion
                  EXPECT_TRUE(Env1.proves(Env1.arena().makeAtomRef(Env1.getFlowConditionToken())));
```

`ASSERT_` aborts the test if the check fails and should generally only be used if it's not meaningful to execute the rest of the test in this case. Otherwise, use `EXPECT_` so that you can continue executing the rest of the test.

It obviously doesn't make a difference in this case, but a) it's good style to default to `EXPECT_` anyway, and b) people might add more checks later, and they might forget to change the `ASSERT_` to an `EXPECT_`.

https://github.com/llvm/llvm-project/pull/80989


More information about the cfe-commits mailing list