martong added a comment.
I don't have anymore immediate concerns, but I will need more time to comb
through the rest of the patch in more details... hopefully I can do that in the
following days.
================
Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:327
+using ObjCForLctxPair =
+ std::pair<const ObjCForCollectionStmt *, const LocationContext *>;
+
----------------
Szelethus wrote:
> martong wrote:
> > Why it is not enough to simply have ObjCForCollectionStmt* as a key?
> `Environment` is the data structure we use to bind **values** to
> **statements** (which would ideally be expressions only). The problem is that
> a statement on its own isn't enough to identify where we are in the analysis:
>
> ```lang=c++
> void traverse(Graph &G) {
> // SubG may need to be modeled in different stackframes, but has the same
> Stmt*
> for (Graph &SubG : G->successors()) {
> traverse(SubG);
> }
> }
> ```
>
> `LocationContext` can be thought of as a stack function calls (or anything
> with a distinct stack frame). Indeed, if two `(Stmt *, LocationContext *)`
> pairs are equal, we're rebinding the same expression (another loop iteration,
> goto), if only the statements are the same, we're not in the same function
> call (//stack frame//).
>
> Of course `ObjCForCollectionStmt` can not have a value, that is the whole
> point of the patch, but we still need a `LocationContext` to correctly
> identify precisely which loop we're talking about.
Yeah, I did not think about stack frames, it makes perfect sense what you
write, thanks for the explanation!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86736/new/
https://reviews.llvm.org/D86736
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
