On Jun 21, 2013, at 6:19 PM, Eli Friedman <[email protected]> wrote: > On Tue, Jun 18, 2013 at 6:41 PM, John McCall <[email protected]> wrote: > On Jun 12, 2013, at 4:42 PM, Eli Friedman <[email protected]> wrote: >> Take a C++ function like the following: >> >> inline void f() { >> ^{ >> static int i = 0; >> printf("%d\n", ++i); >> }(); >> } >> >> Looks pretty simple, right? Unfortunately, trunk clang doesn't handle this >> correctly. The primary issue here is we haven't defined the correct way to >> mangle the name of "i", and clang's current scheme is unusable because it >> depends on the details of IRGen. (There's also the matter that we don't >> compute the linkage correctly, but that's a simple IRGen bug.) >> >> I'm attaching a patch which expands the scheme we use for mangling lambda >> expressions in this sort of context to also work for block literals. The >> patch is still a bit of a mess: there's a bunch of copy-paste code I still >> need to clean up, I haven't included tests, and it would probably be nice to >> include the parameter types of the block in the mangling. That said, it >> essentially implements everything necessary. > > Unlike lambdas, we never actually need to mangle a block declaration; we > just encounter it as a context for local statics and types. And much like we > do with lambdas, we should just ignore the block as a context and mangle the > static/type as a local declaration within the actual enclosing declaration. > > We discussed this in person; basically, we do in fact mangle lambdas into the > context.
Okay. __block_prefix is still an unnecessarily long thing to put in the mangling, and I don't understand why you need to use a different mangling for an internal-linkage entity; that seems like it adds special cases to demangling for no reason. Why not always "Ub <nonnegative-number> _"? >> Note that this doesn't touch the mangling scheme for the actual function >> definitions for blocks; they don't need to be externally visible, so we >> don't need to change the current mangling. >> >> I'd like some feedback to make sure my patch is actually implementing >> something sane. > > Other than the mangling scheme, this seems reasonable. > >> Any suggestions for a better name than LambdaBlockMangleContext are also >> welcome. > > How about ClosureOwningContext? > > I ended up choosing MangleNumberingContext, given that we might need to add > more to it in the future (e.g. static local variables). Static locals and anonymous types in general. > Attaching updated patch; this fixes some details like avoiding some of the > code duplication, fixing the mangling so it makes a bit more sense, and > includes some fixes to make our linkage computations for static local > variables a bit more accurate. Looks good. John.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
