Patch to fix IRGen crash using lambda expression as value in dictionary literal

[Reid Kleckner]

I mean, there's got to be a way to inject some destructor cleanups into the
lambda, like:

struct A { ~A(); };
int f(A) { return 42; }
id test_dict()
{
        return @{
                @"a": [x=f(A())]() { return x; },
                @"b": [x=f(A())]() { return x; }
        };
}

Does this change affect where ~A gets called?

On Tue, Sep 16, 2014 at 5:04 PM, jahanian <fjahanian at apple.com> wrote:

>
> On Sep 16, 2014, at 4:46 PM, Reid Kleckner <rnk at google.com> wrote:
>
> This will have the observable effect of making cleanups run sooner, right?
> Can you test for that?
>
>
> Yes. Generally, it now happens at the end of the ‘value’ part dictionary
> literal evaluation. While before it was happening at where
> expression was considered a full expression (on return statement in this
> example). I am not sure if this change meshes well with
> rules related to clean up code.
>
> For this working test case:
> id test()
> {
>    int cap_a, cap_b;
>         return @{
>                 @"a": ^(){ cap_a; },
>                 @"b": ^(){ cap_b;}
>         };
> }
>
> Before:
>        `-ReturnStmt 0x7fde82076498 <line:20:9, line:23:9>
>       `-ExprWithCleanups 0x7fde82076470 <line:20:16, line:23:9> 'id':'id'
>         |-cleanup Block 0x7fde82076080
>         |-cleanup Block 0x7fde82076270
>
> With this patch:
>
>     `-ReturnStmt 0x7ff2a10766a0 <line:20:9, line:23:9>
>       `-ImplicitCastExpr 0x7ff2a1076688 <line:20:16, line:23:9> 'id':'id'
> <BitCast>
>         `-ObjCDictionaryLiteral 0x7ff2a1076638 <line:20:16, line:23:9>
> 'NSDictionary *'
>           |-ImplicitCastExpr 0x7ff2a1076580 <line:21:17, col:18> 'const
> id<NSCopying>' <BitCast>
>           | `-ObjCStringLiteral 0x7ff2a1076258 <col:17, col:18> 'NSString
> *'
>           |   `-StringLiteral 0x7ff2a1076200 <col:18> 'const char [2]'
> lvalue "a"
>           |-ExprWithCleanups 0x7ff2a10765b0 <col:23, col:35> 'const
> id':'const id'
>           | |-cleanup Block 0x7ff2a1076280
>           | |-cleanup Block 0x7ff2a1076470
>
>
> - fariborz
>
>
>
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s
> -fexceptions -std=c++11 -fblocks
>
> If you aren't checking the IR, use -emit-llvm-only to avoid writing to
> stdout. However, IMO you should be checking the IR here.
>
> On Tue, Sep 16, 2014 at 3:56 PM, jahanian <fjahanian at apple.com> wrote:
>
>> Hi John,
>>
>> This test case:
>>
>> id test_dict()
>> {
>>         return @{
>>                 @"a": [](){},
>>                 @"b": [](){}
>>         };
>> }
>>
>> Crashes in IRGen because ExprWithCleanups for the dummy BlockDecl we use
>> to facilitate lambda to block conversion
>> is at the wrong place.
>> This happens because a lambda expression is considered a full expression
>> so it calls MaybeCreateExprWithCleanups
>> (see Sema::BuildBlockForLambdaConversion).
>> which ends up adding the clean up code for the BlockDecl of the first
>> lambda to the expression for the 2nd lambda!
>>
>> Ideally, MaybeCreateExprWithCleanups should be called when the entire
>> dictionary literal is seen. But in this case,
>> this does not happen. Attached patch fixes this by making sure that after
>> ‘every’ parse of the ‘value’ expression
>> MaybeCreateExprWithCleanups is called. This, in effect, assumes that the
>> ‘value’ literal is a full expression.
>> I don’t know the ramification of this change in the common case as couple
>> of non-lambda literal tests broke.
>> (it now generates the none-exc version of call instead of exc versions in
>> couple of places).
>>
>> Please review. Let me know if you need additional info.
>>
>> - Thanks, Fariborz
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140916/7975b7d5/attachment.html>