[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via llvm-commits]

NoQ added a comment.

I poked Devin offline and we agreed that the overall approach is good to go. Maxim, thank you for picking it up!

We still don't have scopes for segments of code that don't have any variables in them, so i guess it's not yet in the shape where it is super useful for loops, but it's already useful for finding use of stale stack variables, which was the whole point originally, so i think this should definitely land soon.

In https://reviews.llvm.org/D16403#993512, @m.ostapenko wrote:

> In https://reviews.llvm.org/D16403#992452, @NoQ wrote:
>
> > Am i understanding correctly that while destroying `a` you can still use the storage of `b` safely? Or should `a` go out of scope before `b` gets destroyed?
>
>
> AFAIU, when we destroying `a` we can still use the storage of `b` because nothing can be allocated into b's storage between calling destructors for `b` and `a`. So, imho the sequence should look like:
>
>   [B4.5].~A() (Implicit destructor)
>   [B5.3].~A() (Implicit destructor)
>   CFGScopeEnd(b)
>   CFGScopeEnd(a)
>  
>


Thought about it a bit more and this still doesn't look correct to me. Like, `a.~A()` is a function call. It can do a lot of unexpected stuff to registers and stack space before jumping into the function. And given that `a` and `b` are in different scopes (`a` is in loop scope, `b` is in single iteration scope), storage of `b` is not protected from such stuff during call to the destructor of `a`. So there's definitely something fishy here. I guess scope ends and destructors would have to be properly interleaved in all cases of exiting multiple scopes.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403