r303714 - [coroutines] Fix leak in CGCoroutine.cpp

[David Blaikie via cfe-commits]

Fair enough. I don't have all the context there either.

Perhaps Richard Smith could sanity check what the right memory management
scheme is here.

On Mon, May 29, 2017 at 3:54 PM Gor Nishanov <gornishanov at gmail.com> wrote:

> My clang-foo is not strong enough :) .
>
> I wanted to make sure that: "CurCoro.Data->FinalJD =
> getJumpDestInCurrentScope(FinalBB);" is in the correct scope where I want
> it. As opposed to creating it in whatever scope co_return statement is
> encountered. But that could be simply my misundersanding of the
> frontend/scopes/etc and it is quite possible that the code can be
> refactored along the lines you are thinking about.
>
> On Mon, May 29, 2017 at 3:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, May 29, 2017 at 2:12 PM Gor Nishanov <gornishanov at gmail.com>
>> wrote:
>>
>>> It is not known in advance whether the final block is needed or not. It
>>> will become known once the user-authored body of the coroutine is emitted.
>>> I cannot defer creation of it up until that point, since final bb acts as a
>>> jump target for co_returns which could be in the user authored body and I
>>> need it to set up a jump destination beforehand.
>>>
>>
>> Not sure I follow - presumably those jumps could only be emitted if the
>> block would be emitted - so if the entity is created the first time it's
>> needed as a jump destination,then that should be OK?
>>
>>
>>>
>>> Ack on formatting change.
>>>
>>> On Mon, May 29, 2017 at 12:11 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> Could you avoid creating the FinalBB unless it's needed rather than
>>>> creating it and then adding it so it can be removed? (or, if the creation
>>>> can't be avoided, maybe it's OK to 'delete FinalBB' here, rather than
>>>> adding it for it to be removed later?)
>>>>
>>>> (also bracing seems off - could you run your changes through
>>>> clang-format? I'd expect '} else {' on the same line.
>>>>
>>>> On Tue, May 23, 2017 at 6:54 PM Gor Nishanov via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: gornishanov
>>>>> Date: Tue May 23 20:54:37 2017
>>>>> New Revision: 303714
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=303714&view=rev
>>>>> Log:
>>>>> [coroutines] Fix leak in CGCoroutine.cpp
>>>>>
>>>>> FinalBB need to be emitted even when unused to make sure it is deleted
>>>>>
>>>>> Modified:
>>>>>     cfe/trunk/lib/CodeGen/CGCoroutine.cpp
>>>>>
>>>>> Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303714&r1=303713&r2=303714&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
>>>>> +++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Tue May 23 20:54:37 2017
>>>>> @@ -430,6 +430,10 @@ void CodeGenFunction::EmitCoroutineBody(
>>>>>        CurCoro.Data->CurrentAwaitKind = AwaitKind::Final;
>>>>>        EmitStmt(S.getFinalSuspendStmt());
>>>>>      }
>>>>> +    else {
>>>>> +      // We don't need FinalBB. Emit it to make sure the block is
>>>>> deleted.
>>>>> +      EmitBlock(FinalBB, /*IsFinished=*/true);
>>>>> +    }
>>>>>    }
>>>>>
>>>>>    EmitBlock(RetBB);
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170529/45f144a1/attachment.html>