[PATCH] D15031: CFG: Add CFGElement for automatic variables that leave the scope

[Matthias Gehre via cfe-commits]

mgehre added inline comments.

================
Comment at: include/clang/Analysis/CFG.h:170
@@ -168,1 +169,3 @@
 
+class CFGAutomaticObjLeavesScope : public CFGElement {
+public:
----------------
rsmith wrote:
> What is this intended to model? There seem to be a few options here:
> 
>  1) The point at which the destructor would run if the object had a non-trivial destructor
>  2) The point at which the storage duration for the underlying storage ends
>  3) The point at which the lifetime of the object ends
> 
> Note that these are all different -- for an object with a non-trivial destructor, (1) and (3) are the same and for any other object (2) and (3) are the same; (2) occurs after all destructors for the scope have run.
> 
> This matters for cases like:
> 
>   void f() {
>     struct A { int *p; ~A() { *p = 0; } } a;
>     int n;
>     a.p = &n;
>   }
> 
> Here, the lifetime of `n` would end before the lifetime of `a` if `n` had a non-trivial destructor. If `n`s lifetime actually ended there, this code would have undefined behavior. But because the destruction of `n` is trivial, the lifetime of `n` instead ends when its storage duration ends, which is *after* `A`'s destructor runs, so the code is valid.
> 
> Please add a comment to this class to describe what it means.
The intend is to show the point after which it is undefined behavior to refer to an object, i.e.
   3. The point at which the lifetime of the object ends
I will add a comment to clarify this. I propose to rename the class to "CFGLifetimeEnds", okay?

================
Comment at: include/clang/Analysis/CFG.h:728
@@ +727,3 @@
+
+  // Scope leaving must be performed in reversed order. So insertion is in two
+  // steps. First we prepare space for some number of elements, then we insert
----------------
rsmith wrote:
> Why must reverse order be used? It's not possible for the effects of these operations to interact with each other, is it? At least according to the C++ standard, the "end of storage duration" effects all occur simultaneously.
The order is important as you showed in your example above. The liftetime checker should be able to detect the undefined behavior in

```
struct B { B() {}; int n; };
void f() {
  struct A {B *p; ~A() { b->n = 0; } } a;
  B b;
  a.p = &b;
}
```


http://reviews.llvm.org/D15031