[clang][dataflow] Allow analyzing multiple functions in unit tests

[gribozavr]

Summary

In unit tests for concrete dataflow analyses we typically use the
testonly checkDataflow() helper to analyse a free function called
"target". This pattern allows our tests to be uniform and focused on
specific statement- or expression-level C++ features.

As we expand our feature coverage, we want to analyze functions whose
names we don't fully control, like constructors, destructors, operators
etc. In such tests it is often convenient to analyze all functions
defined in the input code, to avoid having to carefully craft an AST
matcher that finds the exact function we're interested in. That can be
easily done by providing checkDataflow() with a catch-all matcher like
functionDecl().

It is also often convenient to define multiple special member functions
in a single unit test, for example, multiple constructors, and share the
rest of the class definition code between constructors. As a result, it
makes sense to analyze multiple functions in one unit test.

This change allows checkDataflow() to correctly handle AST matchers
that match more than one function. Previously, it would only ever
analyze the first matched function, and silently ignore the rest. Now it
runs dataflow analysis in a loop, and calls VerifyResults for each
function that was found in the input and analyzed.

[sgatev]

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

92–107

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

184–185

"bodies of all functions that match"

277–278

"bodies of all functions that match"

304–305

"bodies of all functions that match"

378

Can you please add a comment to describe why this needs to be cleared?

[ymandel]

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

62

This seems subtly wrong if BoundingRange is a TokenRange. It might be ok, if a comment can't be part of a token (at least, as a prefix), but still seems conceptually wrong in that it's not quite doing what it appears. So, if token ranges are ok, I'd recommend at least explaining that in the comments.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

178–179

please specify (in the comments) the intended interpretation of SourceRange -- CharRange or TokenRange -- or use CharSourceRange.

[gribozavr]

Address review comments

[gribozavr2]

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

62

I think it does not matter what kind of range the input range is. The difference is whether the end location is pointing at the beginning or at the end of a token. This span can't contain comments.

Nevertheless, that seemed like subtle reasoning, so I documented the input as a token range, and added a conversion to character range to remove the need for such a subtle justification.

92–107

Applied.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

178–179

Documented as a token range, since that's what callers will most likely have.

184–185

Done.

277–278

Done.

378

Added.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h

template <typename AnalysisT> struct AnalysisInputs {
   AnalysisInputs<AnalysisT> &&
   withBuiltinOptions(DataflowAnalysisContext::Options Options) && {
     BuiltinOptions = std::move(Options);
     return std::move(*this);
   }
 
   /// Required. Input code that is analyzed.
   llvm::StringRef Code;
-  /// Required. The body of the function which matches this matcher is analyzed.
+  /// Required. All functions that match this matcher are analyzed.
   ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher;
   /// Required. The analysis to be run is constructed with this function that
   /// takes as argument the AST generated from the code being analyzed and the
   /// initial state from which the analysis starts with.
   std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis;
   /// Optional. If provided, this function is executed immediately before
   /// running the dataflow analysis to allow for additional setup. All fields in
   /// the `AnalysisOutputs` argument will be initialized except for the
template <typename AnalysisT> struct AnalysisInputs {
   DataflowAnalysisContext::Options BuiltinOptions;
 };
 
 /// Returns assertions based on annotations that are present after statements in
 /// `AnnotatedCode`.
 llvm::Expected<llvm::DenseMap<const Stmt *, std::string>>
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
                                   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AnnotatedCode`.
-llvm::DenseMap<unsigned, std::string>
-buildLineToAnnotationMapping(SourceManager &SM,
-                             llvm::Annotations AnnotatedCode);
-
-/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
-/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`.
-/// Given the analysis outputs, `VerifyResults` checks that the results from the
-/// analysis are correct.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`
+/// within the token range `BoundingRange`.
+llvm::DenseMap<unsigned, std::string> buildLineToAnnotationMapping(
+    const SourceManager &SM, const LangOptions &LangOpts,
+    SourceRange BoundingRange, llvm::Annotations AnnotatedCode);
+
+/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all
+/// functions that match `AI.TargetFuncMatcher` in `AI.Code`.  Given the
+/// analysis outputs, `VerifyResults` checks that the results from the analysis
+/// are correct.
 ///
 /// Requirements:
 ///
 ///   `AnalysisT` contains a type `Lattice`.
 ///
 ///   `Code`, `TargetFuncMatcher` and `MakeAnalysis` must be provided in `AI`.
 ///
 ///   `VerifyResults` must be provided.
checkDataflow(AnalysisInputs<AnalysisT> AI,
   auto &Context = Unit->getASTContext();
 
   if (Context.getDiagnostics().getClient()->getNumErrors() != 0) {
     return llvm::make_error<llvm::StringError>(
         llvm::errc::invalid_argument, "Source file has syntax or type errors, "
                                       "they were printed to the test log");
   }
 
-  // Get AST node of target function.
-  const FunctionDecl *Target = ast_matchers::selectFirst<FunctionDecl>(
-      "target", ast_matchers::match(
-                    ast_matchers::functionDecl(ast_matchers::isDefinition(),
-                                               AI.TargetFuncMatcher)
+  std::function<void(const CFGElement &,
+                     const TypeErasedDataflowAnalysisState &)>
+      TypeErasedPostVisitCFG = nullptr;
+  if (AI.PostVisitCFG) {
+    TypeErasedPostVisitCFG = [&AI, &Context](
+                                 const CFGElement &Element,
+                                 const TypeErasedDataflowAnalysisState &State) {
+      AI.PostVisitCFG(Context, Element,
+                      TransferStateForDiagnostics<typename AnalysisT::Lattice>(
+                          llvm::any_cast<const typename AnalysisT::Lattice &>(
+                              State.Lattice.Value),
+                          State.Env));
+    };
+  }
+
+  for (const ast_matchers::BoundNodes &BN :
+       ast_matchers::match(ast_matchers::functionDecl(
+                               ast_matchers::hasBody(ast_matchers::stmt()),
                                .bind("target"),
-                    Context));
+                           Context)) {
+    // Get the AST node of the target function.
+    const FunctionDecl *Target = BN.getNodeAs<FunctionDecl>("target");
     if (Target == nullptr)
       return llvm::make_error<llvm::StringError>(
-        llvm::errc::invalid_argument, "Could not find target function.");
+          llvm::errc::invalid_argument, "Could not find the target function.");
 
-  // Build control flow graph from body of target function.
+    // Build the control flow graph for the target function.
     auto MaybeCFCtx =
         ControlFlowContext::build(Target, *Target->getBody(), Context);
-  if (!MaybeCFCtx)
-    return MaybeCFCtx.takeError();
+    if (!MaybeCFCtx) return MaybeCFCtx.takeError();
     auto &CFCtx = *MaybeCFCtx;
 
     // Initialize states for running dataflow analysis.
     DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>(),
                                   {/*Opts=*/AI.BuiltinOptions});
     Environment InitEnv(DACtx, *Target);
     auto Analysis = AI.MakeAnalysis(Context, InitEnv);
-  std::function<void(const CFGElement &,
-                     const TypeErasedDataflowAnalysisState &)>
-      PostVisitCFGClosure = nullptr;
-  if (AI.PostVisitCFG) {
-    PostVisitCFGClosure = [&AI, &Context](
-                              const CFGElement &Element,
-                              const TypeErasedDataflowAnalysisState &State) {
-      AI.PostVisitCFG(Context, Element,
-                      TransferStateForDiagnostics<typename AnalysisT::Lattice>(
-                          llvm::any_cast<const typename AnalysisT::Lattice &>(
-                              State.Lattice.Value),
-                          State.Env));
-    };
-  }
 
-  // Additional test setup.
     AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
                        Analysis,      InitEnv, {}};
+
+    // Additional test setup.
     if (AI.SetupTest) {
-    if (auto Error = AI.SetupTest(AO))
-      return Error;
+      if (auto Error = AI.SetupTest(AO)) return Error;
     }
 
     // If successful, the dataflow analysis returns a mapping from block IDs to
     // the post-analysis states for the CFG blocks that have been evaluated.
-  llvm::Expected<std::vector<std::optional<TypeErasedDataflowAnalysisState>>>
-      MaybeBlockStates = runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv,
-                                                       PostVisitCFGClosure);
-  if (!MaybeBlockStates)
-    return MaybeBlockStates.takeError();
+    llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
+        MaybeBlockStates = runTypeErasedDataflowAnalysis(
+            CFCtx, Analysis, InitEnv, TypeErasedPostVisitCFG);
+    if (!MaybeBlockStates) return MaybeBlockStates.takeError();
     AO.BlockStates = *MaybeBlockStates;
 
     // Verify dataflow analysis outputs.
     VerifyResults(AO);
+  }
+
   return llvm::Error::success();
 }
 
-/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
-/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. Given
-/// the annotation line numbers and analysis outputs, `VerifyResults` checks
-/// that the results from the analysis are correct.
+/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all
+/// functions that match `AI.TargetFuncMatcher` in `AI.Code`. Given the
+/// annotation line numbers and analysis outputs, `VerifyResults` checks that
+/// the results from the analysis are correct.
 ///
 /// Requirements:
 ///
 ///   `AnalysisT` contains a type `Lattice`.
 ///
 ///   `Code`, `TargetFuncMatcher` and `MakeAnalysis` must be provided in `AI`.
 ///
 ///   `VerifyResults` must be provided.
 template <typename AnalysisT>
 llvm::Error
 checkDataflow(AnalysisInputs<AnalysisT> AI,
               std::function<void(const llvm::DenseMap<unsigned, std::string> &,
                                  const AnalysisOutputs &)>
                   VerifyResults) {
   return checkDataflow<AnalysisT>(
       std::move(AI), [&VerifyResults](const AnalysisOutputs &AO) {
-        auto AnnotationLinesAndContent =
-            buildLineToAnnotationMapping(AO.ASTCtx.getSourceManager(), AO.Code);
+        auto AnnotationLinesAndContent = buildLineToAnnotationMapping(
+            AO.ASTCtx.getSourceManager(), AO.ASTCtx.getLangOpts(),
+            AO.Target->getSourceRange(), AO.Code);
         VerifyResults(AnnotationLinesAndContent, AO);
       });
 }
 
-/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
-/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. Given
-/// the state computed at each annotated statement and analysis outputs,
-/// `VerifyResults` checks that the results from the analysis are correct.
+/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all
+/// functions that match `AI.TargetFuncMatcher` in `AI.Code`. Given the state
+/// computed at each annotated statement and analysis outputs, `VerifyResults`
+/// checks that the results from the analysis are correct.
 ///
 /// Requirements:
 ///
 ///   `AnalysisT` contains a type `Lattice`.
 ///
 ///   `Code`, `TargetFuncMatcher` and `MakeAnalysis` must be provided in `AI`.
 ///
 ///   `VerifyResults` must be provided.
auto PostVisitCFG =
         assert(InsertSuccess);
       };
   return checkDataflow<AnalysisT>(
       std::move(AI)
           .withSetupTest(std::move(SetupTest))
           .withPostVisitCFG(std::move(PostVisitCFG)),
       [&VerifyResults, &AnnotationStates](const AnalysisOutputs &AO) {
         VerifyResults(AnnotationStates, AO);
+
+        // `checkDataflow()` can analyze more than one function.  Reset the
+        // variables to prepare for analyzing the next function.
+        AnnotationStates.clear();
       });
 }
 
 /// Returns the `ValueDecl` for the given identifier.
 ///
 /// Requirements:
 ///
 ///   `Name` must be unique in `ASTCtx`.

clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp

 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Annotations/Annotations.h"
while (NextToken && SourceManager.getFileOffset(NextToken->getLocation()) <
 
     NextToken = Lexer::findNextToken(NextToken->getEndLoc(), SourceManager,
                                      LangOptions);
   }
 
   return true;
 }
 
-llvm::DenseMap<unsigned, std::string>
-test::buildLineToAnnotationMapping(SourceManager &SM,
-                                   llvm::Annotations AnnotatedCode) {
+llvm::DenseMap<unsigned, std::string> test::buildLineToAnnotationMapping(
+    const SourceManager &SM, const LangOptions &LangOpts,
+    SourceRange BoundingRange, llvm::Annotations AnnotatedCode) {
+  CharSourceRange CharBoundingRange =
+      Lexer::getAsCharRange(BoundingRange, SM, LangOpts);
+
   llvm::DenseMap<unsigned, std::string> LineNumberToContent;
   auto Code = AnnotatedCode.code();
   auto Annotations = AnnotatedCode.ranges();
   for (auto &AnnotationRange : Annotations) {
-    auto LineNumber =
-        SM.getPresumedLineNumber(SM.getLocForStartOfFile(SM.getMainFileID())
-                                     .getLocWithOffset(AnnotationRange.Begin));
-    auto Content = Code.slice(AnnotationRange.Begin, AnnotationRange.End).str();
-    LineNumberToContent[LineNumber] = Content;
+    SourceLocation Loc = SM.getLocForStartOfFile(SM.getMainFileID())
+                             .getLocWithOffset(AnnotationRange.Begin);
+    if (SM.isPointWithin(Loc, CharBoundingRange.getBegin(),
+                         CharBoundingRange.getEnd())) {
+      LineNumberToContent[SM.getPresumedLineNumber(Loc)] =
+          Code.slice(AnnotationRange.Begin, AnnotationRange.End).str();
+    }
   }
   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;
test::buildStatementToAnnotationMapping(const FunctionDecl *Func,
   auto &SourceManager = Context.getSourceManager();
 
   for (auto &Match : match(StmtMatcher, *Func->getBody(), Context)) {
     const auto *S = Match.getNodeAs<Stmt>("stmt");
     unsigned Offset = SourceManager.getFileOffset(S->getEndLoc());
     Stmts[Offset] = S;
   }
 
-  unsigned I = 0;
-  auto Annotations = AnnotatedCode.ranges();
+  unsigned FunctionBeginOffset =
+      SourceManager.getFileOffset(Func->getBeginLoc());
+  unsigned FunctionEndOffset = SourceManager.getFileOffset(Func->getEndLoc());
+
+  std::vector<llvm::Annotations::Range> Annotations = AnnotatedCode.ranges();
+  llvm::erase_if(Annotations, [=](llvm::Annotations::Range R) {
+    return R.Begin < FunctionBeginOffset || R.End >= FunctionEndOffset;
+  });
   std::reverse(Annotations.begin(), Annotations.end());
   auto Code = AnnotatedCode.code();
 
+  unsigned I = 0;
   for (auto OffsetAndStmt = Stmts.rbegin(); OffsetAndStmt != Stmts.rend();
        OffsetAndStmt++) {
     unsigned Offset = OffsetAndStmt->first;
     const Stmt *Stmt = OffsetAndStmt->second;
 
     if (I < Annotations.size() && Annotations[I].Begin >= Offset) {
       auto Range = Annotations[I];
 
       if (!isAnnotationDirectlyAfterStatement(Stmt, Range.Begin, SourceManager,
                                               Context.getLangOpts())) {
         return llvm::createStringError(
             std::make_error_code(std::errc::invalid_argument),

clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp

 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace dataflow;
 
 namespace {
 
 using ::clang::ast_matchers::functionDecl;
+using ::clang::ast_matchers::hasAnyName;
 using ::clang::ast_matchers::hasName;
 using ::clang::ast_matchers::isDefinition;
 using ::clang::dataflow::test::AnalysisInputs;
 using ::clang::dataflow::test::AnalysisOutputs;
 using ::clang::dataflow::test::checkDataflow;
 using ::llvm::IsStringMapEntry;
 using ::testing::_;
 using ::testing::IsEmpty;
)",
           [](const llvm::DenseMap<const Stmt *, std::string> &Annotations) {
             ASSERT_EQ(Annotations.size(), static_cast<unsigned int>(1));
             EXPECT_TRUE(isa<ReturnStmt>(Annotations.begin()->first));
             EXPECT_EQ(Annotations.begin()->second, "ok");
           });
 }
 
 void checkDataflow(
-    llvm::StringRef Code, llvm::StringRef Target,
+    llvm::StringRef Code,
+    ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher,
     std::function<
         void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
              const AnalysisOutputs &)>
         Expectations) {
   ASSERT_THAT_ERROR(checkDataflow<NoopAnalysis>(
                         AnalysisInputs<NoopAnalysis>(
-                            Code, hasName(Target),
+                            Code, std::move(TargetFuncMatcher),
                             [](ASTContext &Context, Environment &) {
                               return NoopAnalysis(
                                   Context, /*ApplyBuiltinTransfer=*/false);
                             })
                             .withASTBuildArgs({"-fsyntax-only", "-std=c++17"}),
-                        /*VerifyResults=*/
-                        std::move(Expectations)),
+                        /*VerifyResults=*/std::move(Expectations)),
                     llvm::Succeeded());
 }
 
 TEST(ProgramPointAnnotations, NoAnnotations) {
   ::testing::MockFunction<void(
       const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
       const AnalysisOutputs &)>
       Expectations;
 
   EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1);
 
-  checkDataflow("void target() {}", "target", Expectations.AsStdFunction());
+  checkDataflow("void target() {}", hasName("target"),
+                Expectations.AsStdFunction());
 }
 
 TEST(ProgramPointAnnotations, NoAnnotationsDifferentTarget) {
   ::testing::MockFunction<void(
       const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
       const AnalysisOutputs &)>
       Expectations;
 
   EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1);
 
-  checkDataflow("void fun() {}", "fun", Expectations.AsStdFunction());
+  checkDataflow("void target() {}", hasName("target"),
+                Expectations.AsStdFunction());
 }
 
-TEST(ProgramPointAnnotations, WithCodepoint) {
+TEST(ProgramPointAnnotations, WithProgramPoint) {
   ::testing::MockFunction<void(
       const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
       const AnalysisOutputs &)>
       Expectations;
 
   EXPECT_CALL(
       Expectations,
       Call(UnorderedElementsAre(IsStringMapEntry("program-point", _)), _))
       .Times(1);
 
   checkDataflow(R"cc(void target() {
                        int n;
                        // [[program-point]]
                      })cc",
-                "target", Expectations.AsStdFunction());
+                hasName("target"), Expectations.AsStdFunction());
 }
 
-TEST(ProgramPointAnnotations, MultipleCodepoints) {
+TEST(ProgramPointAnnotations, MultipleProgramPoints) {
   ::testing::MockFunction<void(
       const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
       const AnalysisOutputs &)>
       Expectations;
 
   EXPECT_CALL(Expectations,
               Call(UnorderedElementsAre(IsStringMapEntry("program-point-1", _),
                                         IsStringMapEntry("program-point-2", _)),
                    _))
       .Times(1);
 
   checkDataflow(R"cc(void target(bool b) {
                        if (b) {
                          int n;
                          // [[program-point-1]]
                        } else {
                          int m;
                          // [[program-point-2]]
                        }
                      })cc",
-                "target", Expectations.AsStdFunction());
+                hasName("target"), Expectations.AsStdFunction());
+}
+
+TEST(ProgramPointAnnotations, MultipleFunctionsMultipleProgramPoints) {
+  ::testing::MockFunction<void(
+      const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+      const AnalysisOutputs &)>
+      Expectations;
+
+  EXPECT_CALL(Expectations, Call(UnorderedElementsAre(
+                                     IsStringMapEntry("program-point-1a", _),
+                                     IsStringMapEntry("program-point-1b", _)),
+                                 _))
+      .Times(1);
+
+  EXPECT_CALL(Expectations, Call(UnorderedElementsAre(
+                                     IsStringMapEntry("program-point-2a", _),
+                                     IsStringMapEntry("program-point-2b", _)),
+                                 _))
+      .Times(1);
+
+  checkDataflow(
+      R"cc(
+        void target1(bool b) {
+          if (b) {
+            int n;
+            // [[program-point-1a]]
+          } else {
+            int m;
+            // [[program-point-1b]]
+          }
+        }
+
+        void target2(bool b) {
+          if (b) {
+            int n;
+            // [[program-point-2a]]
+          } else {
+            int m;
+            // [[program-point-2b]]
+          }
+        }
+      )cc",
+      functionDecl(hasAnyName("target1", "target2")),
+      Expectations.AsStdFunction());
 }
 
 } // namespace