ymandel added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:220
+  assert(Body != nullptr);
+  initGlobalVars(*Body, Env);
+
----------------
samestep wrote:
> ymandel wrote:
> > samestep wrote:
> > > ymandel wrote:
> > > > I wonder how this will work between caller and callee. Do we need 
> > > > separate global var state in the frame? If so, maybe mention that as 
> > > > well in the FIXME above.
> > > Could you clarify what you mean? Perhaps I just don't understand exactly 
> > > what is meant by "global vars" here.
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp#L131-L135
> > 
> > ```
> > /// Initializes global storage values that are declared or referenced from
> > /// sub-statements of `S`.
> > // FIXME: Add support for resetting globals after function calls to enable
> > // the implementation of sound analyses.
> > ```
> > Since this already mentions a need to reset after function calls, seemed 
> > relevant here.
> Hmm, OK. I pretty much just pattern-matched from the `Environment` 
> constructor right above this method implementation. Would it be better for me 
> to instead just remove this `initGlobalVars` call for now and replace it with 
> a `FIXME` saying that we'll probably need to call this but it's unclear how 
> exactly to do so?
SGTM. We can note this constraint on models' reference implementations. 
Specifically, that they cannot reference globals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130306

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

Reply via email to