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

[Robinson, Paul]

> From: Eric Christopher [mailto:echristo at gmail.com]
> Sent: Monday, June 17, 2013 2:32 PM
> To: Robinson, Paul
> Cc: Adrian Prantl; 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 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.
> 
> Nadav? :)
> 
> -eric

Because rescopeLabels() is part of LexicalScope and not RunCleanupsScope?
I don't see what there would be to change in ~RunCleanupsScope.
--paulr