NoQ added a comment.
Herald added subscribers: dkrupp, a.sidorin, rnkovacs.

These CFGs make perfect sense to me so far, and i guess this is a must-have. 
Thank you!

> Note: In case of IndirectGotoStmt, it could happen that we generate LoopExit 
> elements even for loops which is not exited by that jump. However, it does 
> not seem to be a problem. This could result that we can not apply some more 
> precise modeling feature (like unrolling and widening) but not any more - as 
> I can see. (Also, this is a rare case.)

This seems a bit scary because if there's no obvious one-to-once correspondence 
between entrances and exits along every path, how would we know when to pop 
loop contexts from the stack of location contexts? Like, if we attach variable 
lifetime checks to the loop exit, would we need to write additional code in the 
analyzer to see if the variable indeed goes out of scope on the current path? 
Could you demonstrate how it works in a test case over 
https://reviews.llvm.org/D41151?



================
Comment at: include/clang/Analysis/CFG.h:172-178
 /// Represents the point where a loop ends.
 /// This element is is only produced when building the CFG for the static
 /// analyzer and hidden behind the 'cfg-loopexit' analyzer config flag.
 ///
-/// Note: a loop exit element can be reached even when the loop body was never
-/// entered.
+/// Note: whenever a loop is entered a loop exit element will be encountered
+/// after leaving it. However, a loop exit element can be reached even when the
+/// loop body was never entered.
----------------
I'd be glad to see more documentation on where does this `CFGElement` appear, 
when, and why:
- At the end of the block within the loop?
- At the beginning of the block after the loop?
- In its own block constructed with the sole purpose of sheltering it?
Same goes for the `CFGLoopEntrace`, i guess.


================
Comment at: test/Analysis/loopexit-cfg-output.cpp:912-914
+// CHECK:       [B5]
+// CHECK-NEXT:   Succs (1): B8
+
----------------
P.S. This is not a regression introduced by your patch, but i'm really curious 
what does this block even do.


https://reviews.llvm.org/D39398



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

Reply via email to