On Jul 9, 2013, at 10:04 AM, Eric Christopher <[email protected]> wrote: > On Tue, Jul 9, 2013 at 2:33 AM, John McCall <[email protected]> wrote: >> On Jun 17, 2013, at 2:32 PM, Eric Christopher <[email protected]> wrote: >>> On Mon, Jun 17, 2013 at 2:29 PM, Robinson, Paul >>> <[email protected]> wrote: >>>>> From: [email protected] [mailto:cfe-commits- >>>>> [email protected]] On Behalf Of Eric Christopher >>>>> Sent: Monday, June 10, 2013 4:05 PM >>>>> To: Adrian Prantl >>>>> Cc: Nadav Rotem; [email protected] >>>>> 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 <[email protected]> >>>>> 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 <[email protected]> >>>>> 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. >> > > *nod* That's fair. In this case we have a cleanup scope and a debug > lexical scope (though not enshrined as such).
Ah, sure. In that case, there's no harm from rescoping labels. If you see any generation difference, it's more likely to be a reordering of the cleanups around jump emission or something similar. >> 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. > > I think this makes sense, but an example would probably illuminate. You can't jump into an expression because there's stuff gettin' evaluated there. >> 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. > > Oddly enough I did. It was called "LexicalScope" for things that > needed a debug lexical scope along with the assorted cleanups. It > seems to have been appropriated though :) Well, I didn't realize the code in this case was so closely aligned with an actual cleanups scope. John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
