[PATCH] D130600: [clang][dataflow] Handle return statements
Stanislav Gatev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 4 05:26:19 PDT 2022
sgatev added inline comments.
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:348
+ auto *Loc = Env.getReturnStorageLocation();
+ assert(Loc != nullptr);
+ // FIXME: Model NRVO.
> sgatev wrote:
> > Let's do `if (Loc == nullptr) return;`
> I don't think we want to do that, right? Shouldn't the `return` storage location always be set? Or is this about the "analyzing fragments rather than full functions" thing we discussed yesterday?
I think it's related. If we are going with always initializing the `return` storage location then I guess at some point we should be able to make `Environment::getReturnStorageLocation` return a reference? In that case I'm fine with keeping the assert around in the meantime.
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:564
+ assert(ReturnLoc != nullptr);
+ Env.setStorageLocation(*S, *ReturnLoc);
> sgatev wrote:
> > We use stable storage locations to ensure convergence. In that spirit, shouldn't we assign `ReturnLoc`'s value to `S`'s storage location instead of changing the storage location? Alternatively, we can pass `S`'s storage location to `pushCall` so that it can store it as `ReturnLoc`.
> Could you clarify how this hurts convergence? My understanding is that `ReturnLoc` here is already stable, so this would make `S`'s storage location stable too.
If I follow correctly, `ReturnLoc` here is the result of `Env.createStorageLocation(ReturnType)` which isn't stable. Each call to `createStorageLocation` returns a fresh storage location.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits