[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-31 Thread weiyi via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e842dd4bd55: [clang][dataflow] Extend transfer functions 
for other `CFGElement`s (authored by wyt).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-30 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 456624.
wyt added a comment.

Replace use of virtual functions with sfinea and CRTP to call transfer 
functions defined by the user.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It ==

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-26 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 456049.
wyt added a comment.

Mark override for virtual transfer function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
  clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -111,7 +111,8 @@
 
   static NonConvergingLattice initialElement() { return {0}; }
 
-  void transfer(const Stmt *S, NonConvergingLattice , Environment ) {
+  void transfer(const Stmt *S, NonConvergingLattice ,
+Environment ) override {
 ++E.State;
   }
 };
@@ -161,7 +162,8 @@
 
   static FunctionCallLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, FunctionCallLattice , Environment ) {
+  void transfer(const Stmt *S, FunctionCallLattice ,
+Environment ) override {
 if (auto *C = dyn_cast(S)) {
   if (auto *F = dyn_cast(C->getCalleeDecl())) {
 E.CalledFunctions.insert(F->getNameInfo().getAsString());
@@ -305,7 +307,7 @@
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment ) {
+  void transfer(const Stmt *S, NoopLattice &, Environment ) override {
 auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
 auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
 
@@ -457,7 +459,7 @@
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment ) {
+  void transfer(const Stmt *S, NoopLattice &, Environment ) override {
 auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
 auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
 
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-26 Thread weiyi via Phabricator via cfe-commits 
wyt added a comment.

@thakis 
Thanks for pointing that out, will revert and recommit - since it also ran into 
some build failures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-26 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

I don't know how you commited this, but the commit message is missing all the 
(great!) details that are here on the review. See here: 
https://github.com/llvm/llvm-project/commit/4b815eb4fde0202434c6 or here: 
https://reviews.llvm.org/rG4b815eb4fde0202434c63

Please update your commit workflow so that the description on phab makes it 
into the commit going forward :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-26 Thread weiyi via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b815eb4fde0: [clang][dataflow] Extend transfer functions 
for other `CFGElement`s (authored by wyt).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 454532.
wyt added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;
 auto *Lattice = llvm::any_cast(
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 454523.
wyt marked 8 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;
 auto *Lattice = llvm::any_cast(
Index:

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread Stanislav Gatev via Phabricator via cfe-commits 
sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  /// code being analysed.
+  virtual void transferCFGElement(const CFGElement *Element, Lattice ,
+  Environment ) {}

Any reason not to call this one `transfer` too?



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:157
 
+// Holds data structures required for running dataflow analysis.
+struct AnalysisContext {

Let's start struct and member comments with `///`.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:184
 ///   All predecessors of `Block` except those with loop back edges must have
-///   already been transferred. States in `BlockStates` that are set to
+///   already been transferred. States in `TC.BlockStates` that are set to
 ///   `llvm::None` represent basic blocks that are not evaluated yet.





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:312
+/// Transfers `State` by evaluating each element in the `Block` based on the
+/// `TC.Analysis` specified.
+///





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:314
+///
+/// Built in transfer functions (if the option for `ApplyBuiltinTransfer` is 
set
+/// by the analysis) will be applied to the element before evaluation by the





Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:328-340
+  switch (Element.getKind()) {
+  case CFGElement::Statement: {
+builtinTransfer(Element.castAs(), State, AC);
+break;
+  }
+  case CFGElement::Initializer: {
+builtinTransfer(Element.castAs(), State);

The level of abstraction for built-in and user code is different in this 
function which makes it hard to follow. Let's move this in a 
`builtinTransfer(const CFGElement &, TypeErasedDataflowAnalysisState &)` 
function that dispatches to one of the other built-in transfer functions. This 
way `transferCFGBlock` won't mix implementation details with the high level 
algorithm.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:341-347
+}
+// User-provided analysis
+AC.Analysis.transferTypeErased(, State.Lattice, State.Env);
+// Post processing
+if (PostVisitCFG) {
+  PostVisitCFG(Element, State);
+}





Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:164-170
+std::function VerifyResults, ArrayRef 
Args,
+const tooling::FileContentMappings  = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-19 Thread weiyi via Phabricator via cfe-commits 
wyt added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
 llvm::Optional>>>
-runDataflowAnalysis(
+runDataflowAnalysisOnCFG(
 const ControlFlowContext , AnalysisT ,

gribozavr2 wrote:
> 
Cannot be renamed currently, as the use between the function that applies to 
`CFGElement` and `CFGStmt` is ambiguous. Added a FIXME to rename after we have 
updated the implementation for users and remove the function that acts on 
`CFGStmt`s.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:83
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,

gribozavr2 wrote:
> 
Same as above, using the same name for the function that applies to 
`CFGElement` and `CFGStmt` is ambiguous.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-18 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 453702.
wyt marked 2 inline comments as done.
wyt added a comment.

Address comments. Unable to rename `check/runDataflowOnCFG` to 
`check/runDataflow` currently due to ambiguous use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> 
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +153,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +196,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +219,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-16 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
 llvm::Optional>>>
-runDataflowAnalysis(
+runDataflowAnalysisOnCFG(
 const ControlFlowContext , AnalysisT ,





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:167
+
+  // Contains the CFG which the analysis is run over.
+  const ControlFlowContext 





Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:81
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 

Please use triple slash for doc comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:83
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-16 Thread weiyi via Phabricator via cfe-commits 
wyt added a reviewer: xazax.hun.
wyt added inline comments.
Herald added a subscriber: rnkovacs.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  virtual void transferCFGElement(const CFGElement *Element, Lattice ,
+  Environment ) {}
+

gribozavr2 wrote:
> Instead of adding virtual function, please continue following the CRTP 
> pattern. Add the expected function declarations in the comment above the 
> class.
> Instead of adding virtual function, please continue following the CRTP 
> pattern. Add the expected function declarations in the comment above the 
> class.

As discussed, moving to a CRTP pattern causes the code to break as they have 
not been implemented by users yet. Added a FIXME to do so after users have been 
migrated to implement transferCFGElement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-16 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 452964.
wyt added a comment.

Add FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,22 @@
   std::vector> 
 };
 
+// Runs dataflow analysis (specified from `MakeAnalysis`) and the `PostVisitCFG`
+// function (if provided) on the body of the function that matches
+// `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks that the
+// results from the analysis are correct.
+//
+// Requirements:
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +120,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +139,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +150,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +193,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +216,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;
 auto *Lattice = llvm::any_cast(
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-16 Thread weiyi via Phabricator via cfe-commits 
wyt updated this revision to Diff 452957.
wyt marked 9 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,22 @@
   std::vector> 
 };
 
+// Runs dataflow analysis (specified from `MakeAnalysis`) and the `PostVisitCFG`
+// function (if provided) on the body of the function that matches
+// `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks that the
+// results from the analysis are correct.
+//
+// Requirements:
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +120,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
-   const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [, ](
+  const CFGElement ,
+  const TypeErasedDataflowAnalysisState ) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +139,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +150,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +193,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +216,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;
 auto *Lattice = llvm::any_cast(
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
---

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-11 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  virtual void transferCFGElement(const CFGElement *Element, Lattice ,
+  Environment ) {}
+

Instead of adding virtual function, please continue following the CRTP pattern. 
Add the expected function declarations in the comment above the class.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
 Lattice  = llvm::any_cast(E.Value);
-static_cast(this)->transfer(Stmt, L, Env);
+transferCFGElement(Element, L, Env);
+





Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:112
+if (Element->getKind() == CFGElement::Statement) {
+  transfer(Element->getAs()->getStmt(), L, Env);
+}

Ditto.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:134
 /// the dataflow analysis cannot be performed successfully. Otherwise, calls
-/// `PostVisitStmt` on each statement with the final analysis results at that
-/// program point.
+/// `PostVisit` on each element with the final analysis results at that program
+/// point.





Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
+   typename AnalysisT::Lattice> &)>
+PostVisit = nullptr) {
+  std::function(), State);
+  }

PTAL at the intended usage of getAs() here: 
llvm-project/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp

That is, it should be used like dyn_cast in if statements.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:260
 
-/// Transfers `State` by evaluating `CfgStmt` in the context of `Analysis`.
-/// `HandleTransferredStmt` (if provided) will be applied to `CfgStmt`, after 
it
-/// is evaluated.
-static void transferCFGStmt(
-const ControlFlowContext ,
-llvm::ArrayRef> 
BlockStates,
-const CFGStmt , TypeErasedDataflowAnalysis ,
-TypeErasedDataflowAnalysisState ,
-std::function
-HandleTransferredStmt) {
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt ,





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:261
+/// Built-in transfer function for `CfgStmt`.
+void builtInTransfer(const CFGStmt ,
+ TypeErasedDataflowAnalysisState ,

"CfgStmt" is violating the style guide casing rules.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:270-271
 
-/// Transfers `State` by evaluating `CfgInit`.
-static void transferCFGInitializer(const CFGInitializer ,
-   TypeErasedDataflowAnalysisState ) {
-  const auto  = *cast(
-  State.Env.getThisPointeeStorageLocation());
+/// Built-in transfer function for `CfgInit`.
+void builtInTransfer(const CFGInitializer ,
+ TypeErasedDataflowAnalysisState ) {

Ditto.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:325
+  case CFGElement::Statement: {
+builtInTransfer(*Element.getAs(), State, TC);
+break;

Should be using castAs().



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329
+  case CFGElement::Initializer: {
+builtInTransfer(*Element.getAs(), State);
+break;

castAs()



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:438
 } // namespace dataflow
-} // namespace clang
+} // namespace clang

Please add the newline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131614/new/

https://reviews.llvm.org/D131614

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-10 Thread weiyi via Phabricator via cfe-commits 
wyt created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,22 @@
   std::vector> 
 };
 
+// TODO:  add example of annotated test code snippet
+//
+// Runs dataflow analysis (specified from `MakeAnalysis`) and the `PostVisit`
+// function (if provided) on the body of the function that matches
+// `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks that the
+// results from the analysis matches the expectations (which can be specified by
+// annotations in the `Code`).
+// Requires: `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisit,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings  = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,14 +120,15 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [, ](
-   const CFGStmt ,
+  std::function
+  PostVisitClosure = nullptr;
+  if (PostVisit != nullptr) {
+PostVisitClosure =
+[, ](const CFGElement ,
const TypeErasedDataflowAnalysisState ) {
-  PostVisitStmt(Context, Stmt, State);
-};
+  PostVisit(Context, Element, State);
+};
   }
 
   llvm::Expected>
@@ -130,7 +139,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto  = *MaybeBlockStates;
@@ -141,6 +150,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings  = {}) {
+
+  std::function
+  PostVisit = nullptr;
+  if (PostVisitStmt != nullptr) {
+PostVisit = [](ASTContext , const CFGElement ,
+ const TypeErasedDataflowAnalysisState ) {
+  if (Element.getKind() == CFGElement::Statement) {
+PostVisitStmt(Context, *Element.getAs(), State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisit,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +192,9 @@
 const tooling::FileContentMappings  = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisit=*/nullptr,
   [](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +215,12 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [,
-   ](const clang::CFGStmt ,
+   ](const clang::CFGElement ,
  const TypeErasedDataflowAnalysisState ) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: extend testing annotations to non statement constructs
+if (Element.getKind() != clang::CFGElement::Statement)
+  return;
+auto It = Annotations.find(Element.getAs()->getStmt());
 if (It == Annotations.end())
   return;
 auto *Lattice = llvm::any_cast(
Index: