gribozavr created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
gribozavr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140859

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp
@@ -15,6 +15,7 @@
 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;
@@ -74,21 +75,21 @@
 }
 
 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());
 }
 
@@ -100,7 +101,8 @@
 
   EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1);
 
-  checkDataflow("void target() {}", "target", Expectations.AsStdFunction());
+  checkDataflow("void target() {}", hasName("target"),
+                Expectations.AsStdFunction());
 }
 
 TEST(ProgramPointAnnotations, NoAnnotationsDifferentTarget) {
@@ -111,10 +113,11 @@
 
   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 &)>
@@ -126,13 +129,13 @@
       .Times(1);
 
   checkDataflow(R"cc(void target() {
-                     int n;
-                     // [[program-point]]
-                   })cc",
-                "target", Expectations.AsStdFunction());
+                       int n;
+                       // [[program-point]]
+                     })cc",
+                hasName("target"), Expectations.AsStdFunction());
 }
 
-TEST(ProgramPointAnnotations, MultipleCodepoints) {
+TEST(ProgramPointAnnotations, MultipleProgramPoints) {
   ::testing::MockFunction<void(
       const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
       const AnalysisOutputs &)>
@@ -145,15 +148,59 @@
       .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());
+                       if (b) {
+                         int n;
+                         // [[program-point-1]]
+                       } else {
+                         int m;
+                         // [[program-point-2]]
+                       }
+                     })cc",
+                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
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -167,9 +167,10 @@
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
                                   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AnnotatedCode`.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`
+/// within `BoundingRange`.
 llvm::DenseMap<unsigned, std::string>
-buildLineToAnnotationMapping(SourceManager &SM,
+buildLineToAnnotationMapping(SourceManager &SM, SourceRange BoundingRange,
                              llvm::Annotations AnnotatedCode);
 
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
@@ -203,35 +204,13 @@
                                       "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)
-                        .bind("target"),
-                    Context));
-  if (Target == nullptr)
-    return llvm::make_error<llvm::StringError>(
-        llvm::errc::invalid_argument, "Could not find target function.");
-
-  // Build control flow graph from body of target function.
-  auto MaybeCFCtx =
-      ControlFlowContext::build(Target, *Target->getBody(), Context);
-  if (!MaybeCFCtx)
-    return MaybeCFCtx.takeError();
-  auto &CFCtx = *MaybeCFCtx;
-
-  // Initialize states for running dataflow analysis.
-  DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>());
-  Environment InitEnv(DACtx, *Target);
-  auto Analysis = AI.MakeAnalysis(Context, InitEnv);
   std::function<void(const CFGElement &,
                      const TypeErasedDataflowAnalysisState &)>
-      PostVisitCFGClosure = nullptr;
+      TypeErasedPostVisitCFG = nullptr;
   if (AI.PostVisitCFG) {
-    PostVisitCFGClosure = [&AI, &Context](
-                              const CFGElement &Element,
-                              const TypeErasedDataflowAnalysisState &State) {
+    TypeErasedPostVisitCFG = [&AI, &Context](
+                                 const CFGElement &Element,
+                                 const TypeErasedDataflowAnalysisState &State) {
       AI.PostVisitCFG(Context, Element,
                       TransferStateForDiagnostics<typename AnalysisT::Lattice>(
                           llvm::any_cast<const typename AnalysisT::Lattice &>(
@@ -240,25 +219,49 @@
     };
   }
 
-  // Additional test setup.
-  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
-                     Analysis,      InitEnv, {}};
-  if (AI.SetupTest) {
-    if (auto Error = AI.SetupTest(AO))
-      return Error;
+  for (const ast_matchers::BoundNodes &BN :
+       ast_matchers::match(ast_matchers::functionDecl(
+                               ast_matchers::hasBody(ast_matchers::stmt()),
+                               AI.TargetFuncMatcher)
+                               .bind("target"),
+                           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 the target function.");
+
+    // Build the control flow graph for the target function.
+    auto MaybeCFCtx =
+        ControlFlowContext::build(Target, *Target->getBody(), Context);
+    if (!MaybeCFCtx) return MaybeCFCtx.takeError();
+    auto &CFCtx = *MaybeCFCtx;
+
+    // Initialize states for running dataflow analysis.
+    DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>());
+    Environment InitEnv(DACtx, *Target);
+    auto Analysis = AI.MakeAnalysis(Context, InitEnv);
+
+    AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+                       Analysis,      InitEnv, {}};
+
+    // Additional test setup.
+    if (AI.SetupTest) {
+      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<llvm::Optional<TypeErasedDataflowAnalysisState>>>
+        MaybeBlockStates = runTypeErasedDataflowAnalysis(
+            CFCtx, Analysis, InitEnv, TypeErasedPostVisitCFG);
+    if (!MaybeBlockStates) return MaybeBlockStates.takeError();
+    AO.BlockStates = *MaybeBlockStates;
+
+    // Verify dataflow analysis outputs.
+    VerifyResults(AO);
   }
 
-  // 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<llvm::Optional<TypeErasedDataflowAnalysisState>>>
-      MaybeBlockStates = runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv,
-                                                       PostVisitCFGClosure);
-  if (!MaybeBlockStates)
-    return MaybeBlockStates.takeError();
-  AO.BlockStates = *MaybeBlockStates;
-
-  // Verify dataflow analysis outputs.
-  VerifyResults(AO);
   return llvm::Error::success();
 }
 
@@ -282,8 +285,8 @@
                   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.Target->getSourceRange(), AO.Code);
         VerifyResults(AnnotationLinesAndContent, AO);
       });
 }
@@ -361,6 +364,7 @@
           .withPostVisitCFG(std::move(PostVisitCFG)),
       [&VerifyResults, &AnnotationStates](const AnalysisOutputs &AO) {
         VerifyResults(AnnotationStates, AO);
+        AnnotationStates.clear();
       });
 }
 
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -6,6 +6,7 @@
 #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"
@@ -46,17 +47,19 @@
 }
 
 llvm::DenseMap<unsigned, std::string>
-test::buildLineToAnnotationMapping(SourceManager &SM,
+test::buildLineToAnnotationMapping(SourceManager &SM, SourceRange BoundingRange,
                                    llvm::Annotations AnnotatedCode) {
   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, BoundingRange.getBegin(),
+                         BoundingRange.getEnd())) {
+      LineNumberToContent[SM.getPresumedLineNumber(Loc)] =
+          Code.slice(AnnotationRange.Begin, AnnotationRange.End).str();
+    }
   }
   return LineNumberToContent;
 }
@@ -83,8 +86,15 @@
     Stmts[Offset] = S;
   }
 
+  unsigned FunctionBeginOffset =
+      SourceManager.getFileOffset(Func->getBeginLoc());
+  unsigned FunctionEndOffset = SourceManager.getFileOffset(Func->getEndLoc());
+
   unsigned I = 0;
-  auto Annotations = AnnotatedCode.ranges();
+  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();
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D140859: [clang... Dmitri Gribenko via Phabricator via cfe-commits

Reply via email to