[clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

[bazuzi]

Summary

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

[ymandel]

Thanks and welcome! :)

[bazuzi]

Thanks! Seems like you have commit access, Yitzie; could you commit this for me as "Samira Bazuzi <[e-mail]>"?

[ymandel]

In D149464#4306226, @bazuzi wrote:

Thanks! Seems like you have commit access, Yitzie; could you commit this for me as "Samira Bazuzi <[e-mail]>"?

Sure. Let's wait til Gabor approves, but then I'll go ahead.

[xazax.hun]

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

190

Any reason for a comment as opposed to the deprecated attribute?

[bazuzi]

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

190

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.

[xazax.hun]

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

190

Unfortunately, I am also not 100% sure what is the convention. But I am OK with not providing a FIXIT here. I do not expect too many clients yet.

[bazuzi]

Use LLVM_DEPRECATED in place of comments indicating deprecation.

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

 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/Logger.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <memory>
 #include <type_traits>
 #include <utility>
 
 namespace clang {
 namespace dataflow {
 
public:
   ///
   /// If `DeclCtx` is a function, initializes the environment with symbolic
   /// representations of the function parameters.
   ///
   /// If `DeclCtx` is a non-static member function, initializes the environment
   /// with a symbolic representation of the `this` pointee.
   Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx);
 
+  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions() instead.", "")
   const DataflowAnalysisContext::Options &getAnalysisOptions() const {
     return DACtx->getOptions();
   }
 
+  LLVM_DEPRECATED("Use getDataflowAnalysisContext().arena() instead.", "")
   Arena &arena() const { return DACtx->arena(); }
 
+  LLVM_DEPRECATED("Use getDataflowAnalysisContext().getOptions().Log instead.",
+                  "")
   Logger &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.
   ///
   /// Requirements:
   ///
public:
   /// Creates a `T` (some subclass of `Value`), forwarding `args` to the
   /// constructor, and returns a reference to it.
   ///
   /// The analysis context takes ownership of the created object. The object
   /// will be destroyed when the analysis context is destroyed.
   template <typename T, typename... Args>
   std::enable_if_t<std::is_base_of<Value, T>::value, T &>
   create(Args &&...args) {
-    return arena().create<T>(std::forward<Args>(args)...);
+    return DACtx->arena().create<T>(std::forward<Args>(args)...);
   }
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
   /// `Value`
   AtomicBoolValue &getBoolLiteralValue(bool Value) const {
-    return arena().makeLiteral(Value);
+    return DACtx->arena().makeLiteral(Value);
   }
 
   /// Returns an atomic boolean value.
   BoolValue &makeAtomicBoolValue() const {
-    return arena().create<AtomicBoolValue>();
+    return DACtx->arena().create<AtomicBoolValue>();
   }
 
   /// Returns a unique instance of boolean Top.
   BoolValue &makeTopBoolValue() const {
-    return arena().create<TopBoolValue>();
+    return DACtx->arena().create<TopBoolValue>();
   }
 
   /// Returns a boolean value that represents the conjunction of `LHS` and
   /// `RHS`. Subsequent calls with the same arguments, regardless of their
   /// order, will return the same result. If the given boolean values represent
   /// the same value, the result will be the value itself.
   BoolValue &makeAnd(BoolValue &LHS, BoolValue &RHS) const {
-    return arena().makeAnd(LHS, RHS);
+    return DACtx->arena().makeAnd(LHS, RHS);
   }
 
   /// Returns a boolean value that represents the disjunction of `LHS` and
   /// `RHS`. Subsequent calls with the same arguments, regardless of their
   /// order, will return the same result. If the given boolean values represent
   /// the same value, the result will be the value itself.
   BoolValue &makeOr(BoolValue &LHS, BoolValue &RHS) const {
-    return arena().makeOr(LHS, RHS);
+    return DACtx->arena().makeOr(LHS, RHS);
   }
 
   /// Returns a boolean value that represents the negation of `Val`. Subsequent
   /// calls with the same argument will return the same result.
   BoolValue &makeNot(BoolValue &Val) const {
-    return arena().makeNot(Val);
+    return DACtx->arena().makeNot(Val);
   }
 
   /// Returns a boolean value represents `LHS` => `RHS`. Subsequent calls with
   /// the same arguments, will return the same result. If the given boolean
   /// values represent the same value, the result will be a value that
   /// represents the true boolean literal.
   BoolValue &makeImplication(BoolValue &LHS, BoolValue &RHS) const {
-    return arena().makeImplies(LHS, RHS);
+    return DACtx->arena().makeImplies(LHS, RHS);
   }
 
   /// Returns a boolean value represents `LHS` <=> `RHS`. Subsequent calls with
   /// the same arguments, regardless of their order, will return the same
   /// result. If the given boolean values represent the same value, the result
   /// will be a value that represents the true boolean literal.
   BoolValue &makeIff(BoolValue &LHS, BoolValue &RHS) const {
-    return arena().makeEquals(LHS, RHS);
+    return DACtx->arena().makeEquals(LHS, RHS);
   }
 
   /// Returns the token that identifies the flow condition of the environment.
   AtomicBoolValue &getFlowConditionToken() const { return *FlowConditionToken; }
 
   /// Adds `Val` to the set of clauses that constitute the flow condition.
   void addToFlowCondition(BoolValue &Val);
 
   /// Returns true if and only if the clauses that constitute the flow condition
   /// imply that `Val` is true.
   bool flowConditionImplies(BoolValue &Val) const;
 
   /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
   /// returns null.
   const DeclContext *getDeclCtx() const { return CallStack.back(); }
 
   /// Returns whether this `Environment` can be extended to analyze the given
   /// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and a
   /// 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 &getDataflowAnalysisContext() const { return *DACtx; }
+
   LLVM_DUMP_METHOD void dump() const;
   LLVM_DUMP_METHOD void dump(raw_ostream &OS) const;
 
 private:
   /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise
   /// return null.
   ///
   /// Recursively initializes storage locations and values until it sees a

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (ArgLoc == nullptr)
       continue;
 
     const VarDecl *Param = *ParamIt;
     auto &Loc = createStorageLocation(*Param);
     setStorageLocation(*Param, Loc);
 
     QualType ParamType = Param->getType();
     if (ParamType->isReferenceType()) {
-      auto &Val = arena().create<ReferenceValue>(*ArgLoc);
+      auto &Val = DACtx->arena().create<ReferenceValue>(*ArgLoc);
       setValue(Loc, Val);
     } else if (auto *ArgVal = getValue(*ArgLoc)) {
       setValue(Loc, *ArgVal);
     } else if (Value *Val = createValue(ParamType)) {
       setValue(Loc, *Val);
     }
   }
 }
if (Type->isBooleanType()) {
     return &makeAtomicBoolValue();
   }
 
   if (Type->isIntegerType()) {
     // FIXME: consider instead `return nullptr`, given that we do nothing useful
     // with integers, and so distinguishing them serves no purpose, but could
     // prevent convergence.
     CreatedValuesCount++;
-    return &arena().create<IntegerValue>();
+    return &DACtx->arena().create<IntegerValue>();
   }
 
   if (Type->isReferenceType() || Type->isPointerType()) {
     CreatedValuesCount++;
     QualType PointeeType = Type->getPointeeType();
     auto &PointeeLoc = createStorageLocation(PointeeType);
 
     if (Visited.insert(PointeeType.getCanonicalType()).second) {
       Value *PointeeVal = createValueUnlessSelfReferential(
           PointeeType, Visited, Depth, CreatedValuesCount);
       Visited.erase(PointeeType.getCanonicalType());
 
       if (PointeeVal != nullptr)
         setValue(PointeeLoc, *PointeeVal);
     }
 
     if (Type->isReferenceType())
-      return &arena().create<ReferenceValue>(PointeeLoc);
+      return &DACtx->arena().create<ReferenceValue>(PointeeLoc);
     else
-      return &arena().create<PointerValue>(PointeeLoc);
+      return &DACtx->arena().create<PointerValue>(PointeeLoc);
   }
 
   if (Type->isRecordType()) {
     CreatedValuesCount++;
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
     for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();
       if (Visited.contains(FieldType.getCanonicalType()))
         continue;
 
       Visited.insert(FieldType.getCanonicalType());
       if (auto *FieldValue = createValueUnlessSelfReferential(
               FieldType, Visited, Depth + 1, CreatedValuesCount))
         FieldValues.insert({Field, FieldValue});
       Visited.erase(FieldType.getCanonicalType());
     }
 
-    return &arena().create<StructValue>(std::move(FieldValues));
+    return &DACtx->arena().create<StructValue>(std::move(FieldValues));
   }
 
   return nullptr;
 }
 
 StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const {
   switch (SP) {
   case SkipPast::None:

clang/lib/Analysis/FlowSensitive/Transfer.cpp

public:
   void VisitCXXDeleteExpr(const CXXDeleteExpr *S) {
     // Empty method.
     // We consciously don't do anything on deletes.  Diagnosing double deletes
     // (for example) should be done by a specific analysis, not by the
     // framework.
   }
 
   void VisitReturnStmt(const ReturnStmt *S) {
-    if (!Env.getAnalysisOptions().ContextSensitiveOpts)
+    if (!Env.getDataflowAnalysisContext().getOptions().ContextSensitiveOpts)
       return;
 
     auto *Ret = S->getRetValue();
     if (Ret == nullptr)
       return;
 
     auto *Val = Env.getValue(*Ret, SkipPast::None);
     if (Val == nullptr)
BoolValue *getLogicOperatorSubExprValue(const Expr &SubExpr) {
     // boolean value for it.
     return &Env.makeAtomicBoolValue();
   }
 
   // If context sensitivity is enabled, try to analyze the body of the callee
   // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
   template <typename E>
   void transferInlineCall(const E *S, const FunctionDecl *F) {
-    const auto &Options = Env.getAnalysisOptions();
+    const auto &Options = 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;
 
     // FIXME: We don't support context-sensitive analysis of recursion, so
     // we should return early here if `F` is the same as the `FunctionDecl`
     // holding `S` itself.
 
     auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

 /// Holds data structures required for running dataflow analysis.
 struct AnalysisContext {
   AnalysisContext(const ControlFlowContext &CFCtx,
                   TypeErasedDataflowAnalysis &Analysis,
                   const Environment &InitEnv,
                   llvm::ArrayRef<std::optional<TypeErasedDataflowAnalysisState>>
                       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(); }
 
   /// Contains the CFG being analyzed.
   const ControlFlowContext &CFCtx;
   /// The analysis to be run.
   TypeErasedDataflowAnalysis &Analysis;

clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

 };
 
 class TestAnalysis : public DataflowAnalysis<TestAnalysis, TestLattice> {
 public:
   using DataflowAnalysis::DataflowAnalysis;
 
   static TestLattice initialElement() { return TestLattice{}; }
   void transfer(const CFGElement &, TestLattice &L, Environment &E) {
-    E.logger().log([](llvm::raw_ostream &OS) { OS << "transfer()"; });
+    E.getDataflowAnalysisContext().getOptions().Log->log(
+        [](llvm::raw_ostream &OS) { OS << "transfer()"; });
     ++L.Elements;
   }
   void transferBranch(bool Branch, const Stmt *S, TestLattice &L,
                       Environment &E) {
-    E.logger().log([&](llvm::raw_ostream &OS) {
+    E.getDataflowAnalysisContext().getOptions().Log->log(
+        [&](llvm::raw_ostream &OS) {
           OS << "transferBranch(" << Branch << ")";
         });
     ++L.Branches;
   }
 };
 
 class TestLogger : public Logger {
 public:
   TestLogger(std::string &S) : OS(S) {}
 

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

llvm::SmallVector<std::string, 3> ASTBuildArgs = {
       "-fsyntax-only", "-fno-delayed-template-parsing",
       "-std=" +
           std::string(LangStandard::getLangStandardForKind(Std).getName())};
   AnalysisInputs<NoopAnalysis> AI(
       Code, hasName(TargetFun),
       [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
                                                           Environment &Env) {
         return NoopAnalysis(
-            C, DataflowAnalysisOptions{UseBuiltinModel
-                                           ? Env.getAnalysisOptions()
-                                           : std::optional<BuiltinOptions>()});
+            C,
+            DataflowAnalysisOptions{
+                UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
       });
   AI.ASTBuildArgs = ASTBuildArgs;
   if (Options.BuiltinOpts)
     AI.BuiltinOptions = *Options.BuiltinOpts;
   ASSERT_THAT_ERROR(
       checkDataflow<NoopAnalysis>(
           std::move(AI),
           /*VerifyResults=*/