r303714 - [coroutines] Fix leak in CGCoroutine.cpp

[Gor Nishanov via cfe-commits]

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/85e8b4d1/attachment.html>