r219696 - CodeGen: correct mangling for blocks

[Reid Kleckner]

On Tue, Oct 14, 2014 at 1:55 PM, Saleem Abdulrasool <abdulras at fb.com> wrote:

>
> On Oct 14, 2014, at 10:56 AM, Reid Kleckner <rnk at google.com> wrote:
>
> > On Tue, Oct 14, 2014 at 10:20 AM, Saleem Abdulrasool <
> compnerd at compnerd.org> wrote:
> > Author: compnerd
> > Date: Tue Oct 14 12:20:14 2014
> > New Revision: 219696
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=219696&view=rev
> > Log:
> > CodeGen: correct mangling for blocks
> >
> > This addresses a regression introduced with SVN r219393.  A block may be
> > contained within another block.  In such a scenario, we would end up
> within a
> > BlockDecl, which is not a NamedDecl (as the names are synthesised).  The
> cast to
> > a NamedDecl of the DeclContext would then assert as the types are
> unrelated.
> >
> > Restore the mangling behaviour to that prior to SVN r219393.  If the
> current
> > block is contained within a BlockDecl, walk up to the parent DeclContext,
> > recursively, until we have a non-BlockDecl.  This is expected to be a
> NamedDecl.
> > Add in a couple of asserts to ensure that the assumption that we only
> encounter
> > a block within a NamedDecl or a BlockDecl.
> >
> > What about nested global blocks or blocks in CapturedDecls?
>
> What do you mean by global blocks here?  I assume you mean blocks at the
> file scope rather than global blocks in reference to Global vs Stack vs
> Heap blocks.  If so, yes, those are broken, Ive not fixed that (yet), but I
> intend to do that soon.  I had these two ready, so I decided to at least
> improve the state rather than leave it as broken as it is currently.  That
> was partially the reason that I added the assertion, so that the assertion
> triggered would be much easier to catch it.


Yeah, file scope. The mangler uses the name mangleGlobalBlock, so that's
what I went with. :)


> > Also, I don't think we need distinct manglings for blocks in different
> Ctor variants. We can probably significantly simplify the API if we always
> mangle the block using the "base" variant of the ctor/dtor.
>
> The change didn’t really touch the ctor/dtor behaviour.  Im afraid that Im
> not really following you here.


I guess I'm saying that mangleCtorBlock's API seems broken by design,
because we shouldn't invent different manglings for the same block across
different ctor variants.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141014/42a6da39/attachment.html>