[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi added a comment.

Since the timeline for being able to use this is dependent not only on commits 
but integrates as well, lets go ahead with this then.

Can someone commit it for me when we're ready?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

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

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi added a comment.

It looks like D153485  changes the context 
for the last few comments significantly. What's the appetite for adding yet 
another child commit to the chain D153485  is 
in that exposes the solver directly? Instead of committing this and having 
D153485  make a breaking change to expose the 
solver directly if that's better in the new context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

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

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:183
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+

sammccall wrote:
> sammccall wrote:
> > FWIW, I'd probably prefer exposing the solver object itself, having all 
> > capabilities exposed directly through DataflowAnalysisContext gives it this 
> > ugly "god object" quality and the places that we want to use it really just 
> > need arena + solver.
> this should be ArrayRef now... sorry for the churn
HEAD says SetVector, so updated to that. Is there another change coming that 
makes it ArrayRef?



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:183
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+

bazuzi wrote:
> sammccall wrote:
> > sammccall wrote:
> > > FWIW, I'd probably prefer exposing the solver object itself, having all 
> > > capabilities exposed directly through DataflowAnalysisContext gives it 
> > > this ugly "god object" quality and the places that we want to use it 
> > > really just need arena + solver.
> > this should be ArrayRef now... sorry for the churn
> HEAD says SetVector, so updated to that. Is there another change coming that 
> makes it ArrayRef?
If all capabilities was more than 1 capability and this function didn't already 
exist, I'd me more inclined to agree. But requiring users to duplicate the body 
of this function seems worse to me than forwarding an API from a member.

I'm noticing as well that the body of this function adds true and !false 
constraints to the provided set, which I hadn't been doing when using a solver 
and for which I can find no documentation indicating that it's necessary. 
Either we should document why that's done and would need to expect users to 
know to do it if we expose the solver only directly, or we should remove that 
from this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

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

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi updated this revision to Diff 536293.
bazuzi added a comment.

Rebase on main to pull in change from DenseSet to SetVector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -175,6 +175,14 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  ///
+  /// Flow conditions are not incorporated, so they may need to be manually
+  /// included in `Constraints` to provide contextually-accurate results, e.g.
+  /// if any definitions or relationships of the values in `Constraints` have
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::SetVector Constraints);
+
 private:
   friend class Environment;
 
@@ -204,13 +212,6 @@
   AtomicBoolValue , llvm::SetVector ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::SetVector Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::SetVector Constraints) {


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -175,6 +175,14 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  ///
+  /// Flow conditions are not incorporated, so they may need to be manually
+  /// included in `Constraints` to provide contextually-accurate results, e.g.
+  /// if any definitions or relationships of the values in `Constraints` have
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::SetVector Constraints);
+
 private:
   friend class Environment;
 
@@ -204,13 +212,6 @@
   AtomicBoolValue , llvm::SetVector ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::SetVector Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::SetVector Constraints) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi updated this revision to Diff 536269.
bazuzi added a comment.

Updated function comment to remove unnecessary repetition and include 
newly-discovered caveat re: flow conditions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153805

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,14 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  ///
+  /// Flow conditions are not incorporated, so they may need to be manually
+  /// included in `Constraints` to provide contextually-accurate results, e.g.
+  /// if any definitions or relationships of the values in `Constraints` have
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +211,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,14 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  ///
+  /// Flow conditions are not incorporated, so they may need to be manually
+  /// included in `Constraints` to provide contextually-accurate results, e.g.
+  /// if any definitions or relationships of the values in `Constraints` have
+  /// been stored in flow conditions.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +211,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi created this revision.
bazuzi added reviewers: ymandel, gribozavr2, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
bazuzi requested review of this revision.
Herald added a project: clang.

This allows for use of the same solver used by the DAC for additional solving 
post-analysis and thus shared use of MaxIterations in WatchedLiteralsSolver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153805

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,13 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  /// Possible outcomes are:
+  /// - `Satisfiable`: A satisfying assignment exists and is returned.
+  /// - `Unsatisfiable`: A satisfying assignment does not exist.
+  /// - `TimedOut`: The search for a satisfying assignment was not completed.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +210,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {


Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -174,6 +174,13 @@
 
   Arena () { return *A; }
 
+  /// Returns the outcome of satisfiability checking on `Constraints`.
+  /// Possible outcomes are:
+  /// - `Satisfiable`: A satisfying assignment exists and is returned.
+  /// - `Unsatisfiable`: A satisfying assignment does not exist.
+  /// - `TimedOut`: The search for a satisfying assignment was not completed.
+  Solver::Result querySolver(llvm::DenseSet Constraints);
+
 private:
   friend class Environment;
 
@@ -203,13 +210,6 @@
   AtomicBoolValue , llvm::DenseSet ,
   llvm::DenseSet );
 
-  /// Returns the outcome of satisfiability checking on `Constraints`.
-  /// Possible outcomes are:
-  /// - `Satisfiable`: A satisfying assignment exists and is returned.
-  /// - `Unsatisfiable`: A satisfying assignment does not exist.
-  /// - `TimedOut`: The search for a satisfying assignment was not completed.
-  Solver::Result querySolver(llvm::DenseSet Constraints);
-
   /// Returns true if the solver is able to prove that there is no satisfying
   /// assignment for `Constraints`
   bool isUnsatisfiable(llvm::DenseSet Constraints) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149869: [clang][dataflow] Remove deprecated pass-through APIs for DataflowAnalysisContext.

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi created this revision.
bazuzi added reviewers: ymandel, gribozavr2, xazax.hun.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
bazuzi requested review of this revision.
Herald added a project: clang.

These were recently deprecated in https://reviews.llvm.org/D149464.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149869

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h


Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -179,18 +179,6 @@
   /// with a symbolic representation of the `this` pointee.
   Environment(DataflowAnalysisContext , const DeclContext );
 
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions() instead.", "")
-  const DataflowAnalysisContext::Options () const {
-return DACtx->getOptions();
-  }
-
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().arena() instead.", "")
-  Arena () const { return DACtx->arena(); }
-
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions().Log instead.",
-  "")
-  Logger () const { return *DACtx->getOptions().Log; }
-
   /// Creates and returns an environment to use for an inline analysis  of the
   /// callee. Uses the storage location from each argument in the `Call` as the
   /// storage location for the corresponding parameter in the callee.
@@ -412,14 +400,6 @@
   /// given `MaxDepth`.
   bool canDescend(unsigned MaxDepth, const DeclContext *Callee) const;
 
-  /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
-  /// returns null.
-  LLVM_DEPRECATED(
-  "Use getDataflowAnalysisContext().getControlFlowContext(F) instead.", "")
-  const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
-return DACtx->getControlFlowContext(F);
-  }
-
   /// Returns the `DataflowAnalysisContext` used by the environment.
   DataflowAnalysisContext () const { return *DACtx; 
}
 


Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -179,18 +179,6 @@
   /// with a symbolic representation of the `this` pointee.
   Environment(DataflowAnalysisContext , const DeclContext );
 
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions() instead.", "")
-  const DataflowAnalysisContext::Options () const {
-return DACtx->getOptions();
-  }
-
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().arena() instead.", "")
-  Arena () const { return DACtx->arena(); }
-
-  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions().Log instead.",
-  "")
-  Logger () const { return *DACtx->getOptions().Log; }
-
   /// Creates and returns an environment to use for an inline analysis  of the
   /// callee. Uses the storage location from each argument in the `Call` as the
   /// storage location for the corresponding parameter in the callee.
@@ -412,14 +400,6 @@
   /// given `MaxDepth`.
   bool canDescend(unsigned MaxDepth, const DeclContext *Callee) const;
 
-  /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
-  /// returns null.
-  LLVM_DEPRECATED(
-  "Use getDataflowAnalysisContext().getControlFlowContext(F) instead.", "")
-  const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
-return DACtx->getControlFlowContext(F);
-  }
-
   /// Returns the `DataflowAnalysisContext` used by the environment.
   DataflowAnalysisContext () const { return *DACtx; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi updated this revision to Diff 518547.
bazuzi added a comment.

Use LLVM_DEPRECATED in place of comments indicating deprecation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149464

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -53,9 +53,10 @@
   [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext ,
   Environment ) {
 return NoopAnalysis(
-C, DataflowAnalysisOptions{UseBuiltinModel
-   ? Env.getAnalysisOptions()
-   : std::optional()});
+C,
+DataflowAnalysisOptions{
+UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
+: std::optional()});
   });
   AI.ASTBuildArgs = ASTBuildArgs;
   if (Options.BuiltinOpts)
Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -37,14 +37,16 @@
 
   static TestLattice initialElement() { return TestLattice{}; }
   void transfer(const CFGElement &, TestLattice , Environment ) {
-E.logger().log([](llvm::raw_ostream ) { OS << "transfer()"; });
+E.getDataflowAnalysisContext().getOptions().Log->log(
+[](llvm::raw_ostream ) { OS << "transfer()"; });
 ++L.Elements;
   }
   void transferBranch(bool Branch, const Stmt *S, TestLattice ,
   Environment ) {
-E.logger().log([&](llvm::raw_ostream ) {
-  OS << "transferBranch(" << Branch << ")";
-});
+E.getDataflowAnalysisContext().getOptions().Log->log(
+[&](llvm::raw_ostream ) {
+  OS << "transferBranch(" << Branch << ")";
+});
 ++L.Branches;
   }
 };
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -168,7 +168,8 @@
   llvm::ArrayRef>
   BlockStates)
   : CFCtx(CFCtx), Analysis(Analysis), InitEnv(InitEnv),
-Log(InitEnv.logger()), BlockStates(BlockStates) {
+Log(*InitEnv.getDataflowAnalysisContext().getOptions().Log),
+BlockStates(BlockStates) {
 Log.beginAnalysis(CFCtx, Analysis);
   }
   ~AnalysisContext() { Log.endAnalysis(); }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -496,7 +496,7 @@
   }
 
   void VisitReturnStmt(const ReturnStmt *S) {
-if (!Env.getAnalysisOptions().ContextSensitiveOpts)
+if (!Env.getDataflowAnalysisContext().getOptions().ContextSensitiveOpts)
   return;
 
 auto *Ret = S->getRetValue();
@@ -863,12 +863,13 @@
   // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
   template 
   void transferInlineCall(const E *S, const FunctionDecl *F) {
-const auto  = Env.getAnalysisOptions();
+const auto  = Env.getDataflowAnalysisContext().getOptions();
 if (!(Options.ContextSensitiveOpts &&
   Env.canDescend(Options.ContextSensitiveOpts->Depth, F)))
   return;
 
-const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+const ControlFlowContext *CFCtx =
+Env.getDataflowAnalysisContext().getControlFlowContext(F);
 if (!CFCtx)
   return;
 
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -381,7 +381,7 @@
 
 QualType ParamType = Param->getType();
 if (ParamType->isReferenceType()) {
-  auto  = arena().create(*ArgLoc);
+  auto  = DACtx->arena().create(*ArgLoc);
   setValue(Loc, Val);
 } else if (auto *ArgVal = getValue(*ArgLoc)) {
   setValue(Loc, *ArgVal);
@@ -707,7 +707,7 @@
 // with integers, and so distinguishing them serves no purpose, but could
   

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:189
 
+  /// Deprecated. Use *getDataflowAnalysisContext().getOptions().Log instead.
   Logger () const { return *DACtx->getOptions().Log; }

xazax.hun wrote:
> Any reason for a comment as opposed to the deprecated attribute? 
I couldn't tell exactly what the conventions were or whether all of the chained 
replacements could be used as the FIX very well.

If you can confirm that `LLVM_DEPRECATED("DataflowAnalysisContext is now 
directly exposed.", "*getDataflowAnalysisContext().getOptions().Log")`, 
`LLVM_DEPRECATED(..., "getDataflowAnalysisContext().arena")`, etc. will provide 
useful fixes, happy to replace with those. Or with replacements just in the MSG 
and "" for FIX otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149464

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

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi added a comment.

Thanks! Seems like you have commit access, Yitzie; could you commit this for me 
as "Samira Bazuzi "?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149464

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

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

[Samira Bazuzi via Phabricator via cfe-commits]

bazuzi created this revision.
bazuzi added reviewers: ymandel, gribozavr2.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
bazuzi requested review of this revision.
Herald added a project: clang.

This will eliminate the need for more pass-through APIs. Also replace 
pass-through usages with this exposure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149464

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -53,9 +53,10 @@
   [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext ,
   Environment ) {
 return NoopAnalysis(
-C, DataflowAnalysisOptions{UseBuiltinModel
-   ? Env.getAnalysisOptions()
-   : std::optional()});
+C,
+DataflowAnalysisOptions{
+UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
+: std::optional()});
   });
   AI.ASTBuildArgs = ASTBuildArgs;
   if (Options.BuiltinOpts)
Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -37,14 +37,16 @@
 
   static TestLattice initialElement() { return TestLattice{}; }
   void transfer(const CFGElement &, TestLattice , Environment ) {
-E.logger().log([](llvm::raw_ostream ) { OS << "transfer()"; });
+E.getDataflowAnalysisContext().getOptions().Log->log(
+[](llvm::raw_ostream ) { OS << "transfer()"; });
 ++L.Elements;
   }
   void transferBranch(bool Branch, const Stmt *S, TestLattice ,
   Environment ) {
-E.logger().log([&](llvm::raw_ostream ) {
-  OS << "transferBranch(" << Branch << ")";
-});
+E.getDataflowAnalysisContext().getOptions().Log->log(
+[&](llvm::raw_ostream ) {
+  OS << "transferBranch(" << Branch << ")";
+});
 ++L.Branches;
   }
 };
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -168,7 +168,8 @@
   llvm::ArrayRef>
   BlockStates)
   : CFCtx(CFCtx), Analysis(Analysis), InitEnv(InitEnv),
-Log(InitEnv.logger()), BlockStates(BlockStates) {
+Log(*InitEnv.getDataflowAnalysisContext().getOptions().Log),
+BlockStates(BlockStates) {
 Log.beginAnalysis(CFCtx, Analysis);
   }
   ~AnalysisContext() { Log.endAnalysis(); }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -496,7 +496,7 @@
   }
 
   void VisitReturnStmt(const ReturnStmt *S) {
-if (!Env.getAnalysisOptions().ContextSensitiveOpts)
+if (!Env.getDataflowAnalysisContext().getOptions().ContextSensitiveOpts)
   return;
 
 auto *Ret = S->getRetValue();
@@ -863,12 +863,13 @@
   // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
   template 
   void transferInlineCall(const E *S, const FunctionDecl *F) {
-const auto  = Env.getAnalysisOptions();
+const auto  = Env.getDataflowAnalysisContext().getOptions();
 if (!(Options.ContextSensitiveOpts &&
   Env.canDescend(Options.ContextSensitiveOpts->Depth, F)))
   return;
 
-const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+const ControlFlowContext *CFCtx =
+Env.getDataflowAnalysisContext().getControlFlowContext(F);
 if (!CFCtx)
   return;
 
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -381,7 +381,7 @@
 
 QualType ParamType = Param->getType();
 if (ParamType->isReferenceType()) {
-  auto  = arena().create(*ArgLoc);
+  auto  = DACtx->arena().create(*ArgLoc);
   setValue(Loc, Val);
 } else if (auto