r183597 - Debug info: An if condition now creates a lexical scope of its own.

[John McCall]

On Jun 17, 2013, at 2:32 PM, Eric Christopher <echristo at gmail.com> wrote:
> On Mon, Jun 17, 2013 at 2:29 PM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
>>> From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
>>> bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
>>> Sent: Monday, June 10, 2013 4:05 PM
>>> To: Adrian Prantl
>>> Cc: Nadav Rotem; cfe-commits at cs.uiuc.edu
>>> Subject: Re: r183597 - Debug info: An if condition now creates a lexical
>>> scope of its own.
>>> 
>>> On Mon, Jun 10, 2013 at 3:56 PM, Adrian Prantl <aprantl at apple.com>
>>> wrote:
>>>> (CC'ing John because he understands the intricacies of LexicalScope
>>> better than I do)
>>>> 
>>>> On the first glimpse LexicalScope appears to be a subclass of
>>> RunCleanupsScope that additionally emits a (DebugInfo-)LexicalScope. But
>>> looking at the destructors it appears that they have slightly different
>>> semantics: ~LexicalScope runs ForceCleanup and ~RunCleanupsScope
>>> apparently doesn't.
>>>> 
>>>> I'm wary that switching to Lexicalscope in
>>> CodeGenFunction::EmitIfStmt() might lead to tricky ARC or EH-related
>>> problems because of that.
>>>> 
>>>> Does anyone have an opinion on that?
>>> 
>>> That bit was added here:
>>> 
>>> commit 495cfa46300979642acde8d93a1f21c9291dac98
>>> Author: Nadav Rotem <nrotem at apple.com>
>>> Date:   Sat Mar 23 06:43:35 2013 +0000
>>> 
>>>    Make clang to mark static stack allocations with lifetime markers
>>> to enable a more aggressive stack coloring.
>>>    Patch by John McCall with help by Shuxin Yang.
>>>    rdar://13115369
>>> 
>>> 
>>> and oddly not to RunCleanupsScope.
>>> 
>>> Nadav?
>>> 
>>> -eric
>> 
>> Looks like ~LexicalScope() just wants RunCleanupsScope::ForceCleanup()
>> to happen before rescopeLabels().  All the right stuff happens in the right
>> order if you change the RunCleanupsScope instance to LexicalScope.
>> (RunCleanupsScope::ForceCleanup() does pretty much exactly the same thing
>> as ~RunCleanupsScope() so it works out.  It could be done in a more obvious
>> way, but functionally they're equivalent.)
>> 
> 
> Agreed. I was just curious why Nadav changed one, but not the other.

You're asking, why does only LexicalScope manage the set of labels to
rescope?  Because RunCleanupsScope is specifically just supposed to
manage the cleanup stack; it's not meant to do every possible thing that
you might want to do with a language-level lexical scope.

Specifically, rescoping labels isn't necessary for an arbitrary bundle of
cleanups (like a full expression) because you can't typically jump into
such things, and permitting that would involve more than just rescoping
some labels.

You really don't need a full LexicalScope here; all of the child
expressions and statements are already being appropriately scoped.
You should just make your own RAII class.

John.