[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:34
+  /// Builds a ControlFlowContext from an AST node. `D` is the function in which
+  /// `S` resides. All arguments must be non-null.
   static llvm::Expected<ControlFlowContext> build(const Decl *D, Stmt *S,
----------------
ymandel wrote:
> sgatev wrote:
> > Can we make them references?
> Turns out I was wrong - `D` can be null. Would that CFG has comments about what its parameters do... We have a test that excercises this (though I can't say why):
> 
> https://github.com/llvm/llvm-project/blob/9a976f36615dbe15e76c12b22f711b2e597a8e51/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp#L70
> 
> The other two -- yes -- but then we'd have to update the various callers as well. I'd rather not do that in this patch, but I'll add the new overload and we can follow up with cleanups in another patch.
> 
> 
Weird. I think we should change the test to pass the decl.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:369
+
+  llvm::DenseMap<const FunctionDecl *, ControlFlowContext> FunctionModels;
 };
----------------
Perhaps `FunctionContexts`?


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:19
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/Debug.h"
----------------
Is this still needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039