[PATCH] D49718: [CodeGen][ObjC] Make block copy/dispose helper function exception-safe.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 24 14:24:43 PDT 2018


rjmccall added a comment.

In https://reviews.llvm.org/D49718#1173883, @ahatanak wrote:

> Address review comments.
>
> I think I should be able to merge pushBlockObjectRelease and enterByrefCleanup, but the BlockFieldFlags that are passed are different. We set BLOCK_FIELD_IS_WEAK in addition to BLOCK_FIELD_IS_BYREF when isObjCGCWeak() returns true and we are emitting the copy/dispose helper functions, but when enterByrefCleanup is called from EmitAutoVarCleanups, only BLOCK_FIELD_IS_BYREF is set.


Heh, okay.  It looks like the runtime doesn't do anything different, but I think it's probably more correct to always pass `BLOCK_FIELD_IS_WEAK` when destroying a `__weak` block in GC modes.

> In https://reviews.llvm.org/D49718#1173068, @rjmccall wrote:
> 
>> I don't think it makes any difference because we shouldn't be emitting cleanup paths when exceptions are disabled.  I don't think there's an intended difference in semantics between those two destructor-cleanup paths, at least.
> 
> 
> OK, I see.
> 
> If it doesn't make any difference, I think I can push NormalAndEHCleanup unconditionally when copying a BlockObject capture instead of pushing NormalCleanup when EH is disabled. I made that change in the updated patch.

Ok.  Thanks, I like how it looks now.


Repository:
  rC Clang

https://reviews.llvm.org/D49718





More information about the cfe-commits mailing list