[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 3 06:11:35 PDT 2022
ymandel marked 4 inline comments as done.
ymandel 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,
> 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):
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.
Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:50
- ControlFlowContext(std::unique_ptr<CFG> Cfg,
+ // `D` must not be null.
+ ControlFlowContext(const Decl *D, std::unique_ptr<CFG> Cfg,
> Can we make it a reference?
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:23
> I'm guessing this change is unrelated to the patch as a whole?
correct. it was a driveby fix (for better or worse).
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:369
+ // Keyed on the function's fully qualified name. No leading "::".
+ llvm::StringMap<ControlFlowContext> FunctionModels;
> Can we make the keys `FunctionDecl *` and use `FunctionDecl::getDefinition` in `getControlFlowContext` to obtain a canonical pointer?
We can for now -- for the purposes of the cache that makes the most sense. I thought of strings because the goal is a map from *specific* (named) functions, for which we won't a priori have the function decls. still, I'll change for now and we can change it back (or to a better implementation) in the follow up patches.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits