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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
