[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked an inline comment as done.
mboehme added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:71
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
 

chapuni wrote:
> This was the only user of Body in -Asserts
Thanks for pointing this out. Fixed in https://reviews.llvm.org/D151430.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[NAKAMURA Takumi via Phabricator via cfe-commits]

chapuni added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:71
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
 

This was the only user of Body in -Asserts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG246626a8cfd3: [clang][dataflow] Add a 
`ControlFlowContext::build()` overload taking a… (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.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
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -241,8 +241,7 @@
   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);
+auto MaybeCFCtx = ControlFlowContext::build(*Target, Context);
 if (!MaybeCFCtx) return MaybeCFCtx.takeError();
 auto  = *MaybeCFCtx;
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -210,8 +210,8 @@
   if (It != FunctionContexts.end())
 return >second;
 
-  if (Stmt *Body = F->getBody()) {
-auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext());
+  if (F->hasBody()) {
+auto CFCtx = ControlFlowContext::build(*F, F->getASTContext());
 // FIXME: Handle errors.
 assert(CFCtx);
 auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -67,6 +67,16 @@
   return BlockReachable;
 }
 
+llvm::Expected
+ControlFlowContext::build(const FunctionDecl , ASTContext ) {
+  if (!Func.hasBody())
+return llvm::createStringError(
+std::make_error_code(std::errc::invalid_argument),
+"Cannot analyze function without a body");
+
+  return build(Func, *Func.getBody(), C);
+}
+
 llvm::Expected
 ControlFlowContext::build(const Decl , Stmt , ASTContext ) {
   if (D.isTemplated())
Index: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -31,6 +31,11 @@
 /// analysis.
 class ControlFlowContext {
 public:
+  /// Builds a ControlFlowContext from a `FunctionDecl`.
+  /// `Func.hasBody()` must be true, and `Func.isTemplated()` must be false.
+  static llvm::Expected build(const FunctionDecl ,
+  ASTContext );
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.
   static llvm::Expected build(const Decl , Stmt ,


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: 

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked an inline comment as done.
mboehme added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:39
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.

xazax.hun wrote:
> I was wondering if there is a plan to make the framework work for 
> non-functions, like global initializers. 
I believe there may be? I remember talking to someone who mentioned this -- I 
don't know, it might have been you?

This is, really, the only reason I can see for having an overload that takes a 
separate `Stmt`. It doesn't really make sense (I think) to pass a 
`FunctionDecl` to this overload and then pass some `Stmt` that isn't the 
complete function body. (I can't think of any good scenarios where the control 
flow wouldn't escape that `Stmt`, and I don't see any good use cases anyway.) 
So I've been assuming that this overload is there for global initializers. 
Confusingly, the comment says that `D` should be a function, but notably, `D` 
is a `Decl`, not a `FunctionDecl` -- so I think the comment is wrong here.

Anyway, I'll try and get some more insights into this, but until then, I'll 
certainly keep this overload in place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:39
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.

I was wondering if there is a plan to make the framework work for 
non-functions, like global initializers. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

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

This is the most common use case, so it makes sense to have a specific overload
for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151183

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.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
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -241,8 +241,7 @@
   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);
+auto MaybeCFCtx = ControlFlowContext::build(*Target, Context);
 if (!MaybeCFCtx) return MaybeCFCtx.takeError();
 auto  = *MaybeCFCtx;
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -210,8 +210,8 @@
   if (It != FunctionContexts.end())
 return >second;
 
-  if (Stmt *Body = F->getBody()) {
-auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext());
+  if (F->hasBody()) {
+auto CFCtx = ControlFlowContext::build(*F, F->getASTContext());
 // FIXME: Handle errors.
 assert(CFCtx);
 auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -67,6 +67,16 @@
   return BlockReachable;
 }
 
+llvm::Expected
+ControlFlowContext::build(const FunctionDecl , ASTContext ) {
+  if (!Func.hasBody())
+return llvm::createStringError(
+std::make_error_code(std::errc::invalid_argument),
+"Cannot analyze function without a body");
+
+  return build(Func, *Func.getBody(), C);
+}
+
 llvm::Expected
 ControlFlowContext::build(const Decl , Stmt , ASTContext ) {
   if (D.isTemplated())
Index: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -31,6 +31,11 @@
 /// analysis.
 class ControlFlowContext {
 public:
+  /// Builds a ControlFlowContext from a `FunctionDecl`.
+  /// `Func.hasBody()` must be true, and `Func.isTemplated()` must be false.
+  static llvm::Expected build(const FunctionDecl ,
+  ASTContext );
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.
   static llvm::Expected build(const Decl , Stmt ,


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: