[clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector<pair<string, StateT>>`.

[wyt]

Summary

Depends On D132377

[gribozavr2]

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

124

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

287–288

330–333

Could you add an assert that insert reported that it indeed inserted an element (didn't overwrite)?

387–388

Would a range-based for loop work?

for (const auto &P : AnnotationStates)

389–392

Would make_pair work? It allows to omit explicit template arguments.

394–396

Please use llvm::sort. You also don't need to specify the comparator, the default comparator for strings is exactly the same.

[wyt]

Address comments.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

394–396

Used llvm::sort.

The default comparator for pairs also tries to compare the second field which is undefined for DataflowAnalysisState. And considering that annotations are unique here, it should be sufficient to just compare the strings here - hence the specified comparator that just checks the first of the pair.

[gribozavr2]

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

332

Please add (void)InsertSuccess; to silence an "unused variable" warning in release builds.

[wyt]

Address comment.

[ymandel]

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

652–653

Given that its now a map, should we use UnorderedElementsAre? Or, does StringMap guarantee ordering?

[wyt]

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

652–653

StringMap is not yet exposed to the tests in this patch.

Some ordering is currently required to correctly retrieve the states in the tests. Hence, the conversion from StringMap to vector<pair<string, StateT>> (in deprecated checkDataflow) orders the elements alphabetically on the annotations.

Future refactoring will expose StringMap in the tests, where we will then not need to maintain an ordering since we can retrieve elements by keys, we can then use UnorderedElementsAre.

[wyt]

Propagate change from parent patch.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Annotations.h"
 
 namespace clang {
 namespace dataflow {
 

 ///
 ///   `VerifyResults` must be provided.
 ///
 ///   Any annotations appearing in `Code` must come after a statement.
 ///
 ///   There can be at most one annotation attached per statement.
 ///
 ///   Annotations must not be repeated.
 template <typename AnalysisT>
-llvm::Error checkDataflow(
-    AnalysisInputs<AnalysisT> AI,
-    std::function<void(
-        llvm::ArrayRef<std::pair<
-            std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
+llvm::Error
+checkDataflow(AnalysisInputs<AnalysisT> AI,
+              std::function<void(const llvm::StringMap<DataflowAnalysisState<
+                                     typename AnalysisT::Lattice>> &,
                                  const AnalysisOutputs &)>
                   VerifyResults) {
   // Compute mapping from nodes of annotated statements to the content in the
   // annotation.
   llvm::DenseMap<const Stmt *, std::string> StmtToAnnotations;
   auto SetupTest = [&StmtToAnnotations,
                     PrevSetupTest = std::move(AI.SetupTest)](
                        AnalysisOutputs &AO) -> llvm::Error {
     auto MaybeStmtToAnnotations = buildStatementToAnnotationMapping(
         cast<FunctionDecl>(AO.InitEnv.getDeclCtx()), AO.Code);
     if (!MaybeStmtToAnnotations) {
       return MaybeStmtToAnnotations.takeError();
     }
     StmtToAnnotations = std::move(*MaybeStmtToAnnotations);
     return PrevSetupTest ? PrevSetupTest(AO) : llvm::Error::success();
   };
 
   using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
 
-  // FIXME: Use a string map instead of a vector of pairs.
-  //
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
-  std::vector<std::pair<std::string, StateT>> AnnotationStates;
+  llvm::StringMap<StateT> AnnotationStates;
   auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
                        PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
                           ASTContext &Ctx, const CFGElement &Elt,
                           const TypeErasedDataflowAnalysisState &State) {
     if (PrevPostVisitCFG) {
       PrevPostVisitCFG(Ctx, Elt, State);
     }
     // FIXME: Extend retrieval of state for non statement constructs.
     auto Stmt = Elt.getAs<CFGStmt>();
     if (!Stmt)
       return;
     auto It = StmtToAnnotations.find(Stmt->getStmt());
     if (It == StmtToAnnotations.end())
       return;
     auto *Lattice =
         llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value);
-    AnnotationStates.emplace_back(It->second, StateT{*Lattice, State.Env});
+    auto [_, InsertSuccess] =
+        AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
+    (void)InsertSuccess;
+    assert(InsertSuccess);
   };
   return checkDataflow<AnalysisT>(
       std::move(AI)
           .withSetupTest(std::move(SetupTest))
           .withPostVisitCFG(std::move(PostVisitCFG)),
       [&VerifyResults, &AnnotationStates](const AnalysisOutputs &AO) {
         VerifyResults(AnnotationStates, AO);
       });
std::function<void(
         VerifyResults,
     ArrayRef<std::string> Args,
     const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   return checkDataflow<AnalysisT>(
       AnalysisInputs<AnalysisT>(Code, std::move(TargetFuncMatcher),
                                 std::move(MakeAnalysis))
           .withASTBuildArgs(std::move(Args))
           .withASTBuildVirtualMappedFiles(std::move(VirtualMappedFiles)),
-      [&VerifyResults](
-          llvm::ArrayRef<std::pair<
-              std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>
-              AnnotationStates,
+      [&VerifyResults](const llvm::StringMap<DataflowAnalysisState<
+                           typename AnalysisT::Lattice>> &AnnotationStates,
                        const AnalysisOutputs &AO) {
-        VerifyResults(AnnotationStates, AO.ASTCtx);
+        std::vector<std::pair<
+            std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>
+            AnnotationStatesAsVector;
+        for (const auto &P : AnnotationStates) {
+          AnnotationStatesAsVector.push_back(
+              std::make_pair(P.first().str(), std::move(P.second)));
+        }
+        llvm::sort(AnnotationStatesAsVector,
+                   [](auto a, auto b) { return a.first < b.first; });
+
+        VerifyResults(AnnotationStatesAsVector, AO.ASTCtx);
       });
 }
 
 // Deprecated.
 // FIXME: Remove this function after usage has been updated to the overload
 // which uses the `AnalysisInputs` struct.
 //
 /// Runs dataflow specified from `MakeAnalysis` on the body of the function
 /// named `TargetFun` in `Code`. Given the state computed at each annotated

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

 #include "clang/Lex/Lexer.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include <cassert>
 #include <functional>
 #include <memory>
 #include <string>
 #include <system_error>
 #include <utility>
test::buildLineToAnnotationMapping(SourceManager &SM,
   }
   return LineNumberToContent;
 }
 
 llvm::Expected<llvm::DenseMap<const Stmt *, std::string>>
 test::buildStatementToAnnotationMapping(const FunctionDecl *Func,
                                         llvm::Annotations AnnotatedCode) {
   llvm::DenseMap<const Stmt *, std::string> Result;
+  llvm::StringSet<> ExistingAnnotations;
 
   auto StmtMatcher =
       findAll(stmt(unless(anyOf(hasParent(expr()), hasParent(returnStmt()))))
                   .bind("stmt"));
 
   // This map should stay sorted because the binding algorithm relies on the
   // ordering of statement offsets
   std::map<unsigned, const Stmt *> Stmts;
if (I < Annotations.size() && Annotations[I].Begin >= Offset) {
             std::make_error_code(std::errc::invalid_argument),
             "Annotation is not placed after a statement: %s",
             SourceManager.getLocForStartOfFile(SourceManager.getMainFileID())
                 .getLocWithOffset(Offset)
                 .printToString(SourceManager)
                 .data());
       }
 
-      Result[Stmt] = Code.slice(Range.Begin, Range.End).str();
+      auto Annotation = Code.slice(Range.Begin, Range.End).str();
+      if (!ExistingAnnotations.insert(Annotation).second) {
+        return llvm::createStringError(
+            std::make_error_code(std::errc::invalid_argument),
+            "Repeated use of annotation: %s", Annotation.data());
+      }
+      Result[Stmt] = std::move(Annotation);
+
       I++;
 
       if (I < Annotations.size() && Annotations[I].Begin >= Offset) {
         return llvm::createStringError(
             std::make_error_code(std::errc::invalid_argument),
             "Multiple annotations bound to the statement at the location: %s",
             Stmt->getBeginLoc().printToString(SourceManager).data());
       }

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

void target(bool B) {
       (void)0;
       // [[p4]]
     }
   )";
   runDataflow(Code, [](llvm::ArrayRef<std::pair<
                            std::string, DataflowAnalysisState<NoopLattice>>>
                            Results,
                        ASTContext &ASTCtx) {
-    ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
-                                     Pair("p2", _), Pair("p1", _)));
+    ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+                                     Pair("p3", _), Pair("p4", _)));
     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 Environment &Env1 = Results[3].second.Env;
+    const Environment &Env1 = Results[0].second.Env;
     const StorageLocation *FooLoc =
         Env1.getStorageLocation(*FooDecl, SkipPast::None);
     EXPECT_THAT(FooLoc, NotNull());
     EXPECT_THAT(Env1.getStorageLocation(*BarDecl, SkipPast::None), IsNull());
     EXPECT_THAT(Env1.getStorageLocation(*BazDecl, SkipPast::None), IsNull());
 
-    const Environment &Env2 = Results[2].second.Env;
+    const Environment &Env2 = Results[1].second.Env;
     EXPECT_EQ(Env2.getStorageLocation(*FooDecl, SkipPast::None), FooLoc);
     EXPECT_THAT(Env2.getStorageLocation(*BarDecl, SkipPast::None), NotNull());
     EXPECT_THAT(Env2.getStorageLocation(*BazDecl, SkipPast::None), IsNull());
 
-    const Environment &Env3 = Results[1].second.Env;
+    const Environment &Env3 = Results[2].second.Env;
     EXPECT_EQ(Env3.getStorageLocation(*FooDecl, SkipPast::None), FooLoc);
     EXPECT_THAT(Env3.getStorageLocation(*BarDecl, SkipPast::None), IsNull());
     EXPECT_THAT(Env3.getStorageLocation(*BazDecl, SkipPast::None), NotNull());
 
-    const Environment &Env4 = Results[0].second.Env;
+    const Environment &Env4 = Results[3].second.Env;
     EXPECT_EQ(Env4.getStorageLocation(*FooDecl, SkipPast::None), FooLoc);
     EXPECT_THAT(Env4.getStorageLocation(*BarDecl, SkipPast::None), IsNull());
     EXPECT_THAT(Env4.getStorageLocation(*BazDecl, SkipPast::None), IsNull());
   });
 }
 
 TEST(TransferTest, BinaryOperatorAssign) {
   std::string Code = R"(
runDataflow(
                    Results,
                ASTContext &ASTCtx) {
         ASSERT_THAT(Results, SizeIs(3));
 
         const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
         ASSERT_THAT(CDecl, NotNull());
 
         {
-          ASSERT_THAT(Results[2], Pair("p0", _));
-          const Environment &Env = Results[2].second.Env;
+          ASSERT_THAT(Results[0], Pair("p0", _));
+          const Environment &Env = Results[0].second.Env;
           const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
           ASSERT_THAT(BDecl, NotNull());
           auto &BVal = *cast<BoolValue>(Env.getValue(*BDecl, SkipPast::None));
 
           EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(BVal)));
         }
 
         {
           ASSERT_THAT(Results[1], Pair("p1", _));
           const Environment &Env = Results[1].second.Env;
           auto &CVal = *cast<BoolValue>(Env.getValue(*CDecl, SkipPast::None));
           EXPECT_TRUE(Env.flowConditionImplies(CVal));
         }
 
         {
-          ASSERT_THAT(Results[0], Pair("p2", _));
-          const Environment &Env = Results[0].second.Env;
+          ASSERT_THAT(Results[2], Pair("p2", _));
+          const Environment &Env = Results[2].second.Env;
           auto &CVal = *cast<BoolValue>(Env.getValue(*CDecl, SkipPast::None));
           EXPECT_TRUE(Env.flowConditionImplies(CVal));
         }
       });
 }
 
 TEST(TransferTest, LoopWithAssignmentConverges) {
   std::string Code = R"(
TEST(TransferTest, LoopWithStructReferenceAssignmentConverges) {
   // namely, that the analysis succeeds, rather than hitting the maximum number
   // of iterations.
   runDataflow(
       Code, [](llvm::ArrayRef<
                    std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                    Results,
                ASTContext &ASTCtx) {
         ASSERT_THAT(Results,
-                    ElementsAre(Pair("p-outer", _), Pair("p-inner", _)));
-        const Environment &OuterEnv = Results[0].second.Env;
-        const Environment &InnerEnv = Results[1].second.Env;
+                    ElementsAre(Pair("p-inner", _), Pair("p-outer", _)));
+        const Environment &InnerEnv = Results[0].second.Env;
+        const Environment &OuterEnv = Results[1].second.Env;
 
         const ValueDecl *ValDecl = findValueDecl(ASTCtx, "val");
         ASSERT_THAT(ValDecl, NotNull());
 
         const ValueDecl *LDecl = findValueDecl(ASTCtx, "l");
         ASSERT_THAT(LDecl, NotNull());
 
         // Inner.

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

void target(bool Cond) {
       /*[[p4]]*/
     }
   )";
   runDataflow(
       Code, [](llvm::ArrayRef<
                    std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                    Results,
                ASTContext &ASTCtx) {
-        ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
-                                         Pair("p2", _), Pair("p1", _)));
-        const Environment &Env1 = Results[3].second.Env;
-        const Environment &Env2 = Results[2].second.Env;
-        const Environment &Env3 = Results[1].second.Env;
-        const Environment &Env4 = Results[0].second.Env;
+        ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+                                         Pair("p3", _), Pair("p4", _)));
+        const Environment &Env1 = Results[0].second.Env;
+        const Environment &Env2 = Results[1].second.Env;
+        const Environment &Env3 = Results[2].second.Env;
+        const Environment &Env4 = Results[3].second.Env;
 
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         auto GetFooValue = [FooDecl](const Environment &Env) {
           return cast<BoolValue>(
               Env.getValue(*FooDecl, SkipPast::None)->getProperty("is_set"));
         };
TEST_F(WideningTest, JoinDistinctValuesWithDistinctProperties) {
   )";
   runDataflow(
       Code,
       [this](llvm::ArrayRef<
                  std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                  Results,
              ASTContext &ASTCtx) {
         ASSERT_THAT(Results,
-                    ElementsAre(Pair("p3", _), Pair("p2", _), Pair("p1", _)));
-        const Environment &Env1 = Results[2].second.Env;
+                    ElementsAre(Pair("p1", _), Pair("p2", _), Pair("p3", _)));
+        const Environment &Env1 = Results[0].second.Env;
         const Environment &Env2 = Results[1].second.Env;
-        const Environment &Env3 = Results[0].second.Env;
+        const Environment &Env3 = Results[2].second.Env;
 
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         auto GetFooValue = [FooDecl](const Environment &Env) {
           return Env.getValue(*FooDecl, SkipPast::None);
         };
 
void target(bool Cond) {
       /*[[p4]]*/
     }
   )";
   runDataflow(Code,
               [](llvm::ArrayRef<
                      std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                      Results,
                  ASTContext &ASTCtx) {
-                ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
-                                                 Pair("p2", _), Pair("p1", _)));
-                const Environment &Env1 = Results[3].second.Env;
-                const Environment &Env2 = Results[2].second.Env;
-                const Environment &Env3 = Results[1].second.Env;
-                const Environment &Env4 = Results[0].second.Env;
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+                                                 Pair("p3", _), Pair("p4", _)));
+                const Environment &Env1 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
+                const Environment &Env3 = Results[2].second.Env;
+                const Environment &Env4 = Results[3].second.Env;
 
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
                 auto GetFooValue = [FooDecl](const Environment &Env) {
                   return Env.getValue(*FooDecl, SkipPast::None);
                 };
 
TEST_F(FlowConditionTest, IfStmtSingleVar) {
   runDataflow(Code,
               [](llvm::ArrayRef<
                      std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
               });
 }
 
 TEST_F(FlowConditionTest, IfStmtSingleNegatedVar) {
   std::string Code = R"(
TEST_F(FlowConditionTest, IfStmtSingleNegatedVar) {
   runDataflow(Code,
               [](llvm::ArrayRef<
                      std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 EXPECT_FALSE(Env1.flowConditionImplies(*FooVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 EXPECT_TRUE(Env2.flowConditionImplies(*FooVal2));
               });
 }
 
 TEST_F(FlowConditionTest, WhileStmt) {
   std::string Code = R"(
runDataflow(Code,
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal1 =
                     cast<BoolValue>(Env1.getValue(*BarDecl, SkipPast::None));
                 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
                 EXPECT_TRUE(Env1.flowConditionImplies(*BarVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal2 =
                     cast<BoolValue>(Env2.getValue(*BarDecl, SkipPast::None));
                 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
                 EXPECT_FALSE(Env2.flowConditionImplies(*BarVal2));
               });
 }
runDataflow(Code,
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal1 =
                     cast<BoolValue>(Env1.getValue(*BarDecl, SkipPast::None));
                 EXPECT_FALSE(Env1.flowConditionImplies(*FooVal1));
                 EXPECT_FALSE(Env1.flowConditionImplies(*BarVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal2 =
                     cast<BoolValue>(Env2.getValue(*BarDecl, SkipPast::None));
                 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
                 EXPECT_FALSE(Env2.flowConditionImplies(*BarVal2));
               });
 }
runDataflow(Code,
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal1 =
                     cast<BoolValue>(Env1.getValue(*BarDecl, SkipPast::None));
                 EXPECT_FALSE(Env1.flowConditionImplies(*FooVal1));
                 EXPECT_FALSE(Env1.flowConditionImplies(*BarVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal2 =
                     cast<BoolValue>(Env2.getValue(*BarDecl, SkipPast::None));
                 EXPECT_TRUE(Env2.flowConditionImplies(*FooVal2));
                 EXPECT_TRUE(Env2.flowConditionImplies(*BarVal2));
               });
 }
runDataflow(Code,
                      Results,
                  ASTContext &ASTCtx) {
                 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
                 ASSERT_THAT(FooDecl, NotNull());
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
-                ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+                ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-                const Environment &Env1 = Results[1].second.Env;
+                const Environment &Env1 = Results[0].second.Env;
                 auto *FooVal1 =
                     cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal1 =
                     cast<BoolValue>(Env1.getValue(*BarDecl, SkipPast::None));
                 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
                 EXPECT_TRUE(Env1.flowConditionImplies(*BarVal1));
 
-                const Environment &Env2 = Results[0].second.Env;
+                const Environment &Env2 = Results[1].second.Env;
                 auto *FooVal2 =
                     cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::None));
                 auto *BarVal2 =
                     cast<BoolValue>(Env2.getValue(*BarDecl, SkipPast::None));
                 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
                 EXPECT_FALSE(Env2.flowConditionImplies(*BarVal2));
               });
 }
void target(int *Ptr) {
       /*[[p2]]*/
     }
   )";
   runDataflow(
       Code, [](llvm::ArrayRef<
                    std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
                    Results,
                ASTContext &ASTCtx) {
-        ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+        ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
-        const Environment &Env1 = Results[1].second.Env;
+        const Environment &Env1 = Results[0].second.Env;
         auto &FooVal1 =
             *cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::Reference));
         EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
 
-        const Environment &Env2 = Results[0].second.Env;
+        const Environment &Env2 = Results[1].second.Env;
         auto &FooVal2 =
             *cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::Reference));
         EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
       });
 }
 
 } // namespace