================
@@ -2764,6 +2794,9 @@ void CodeGenFunction::emitByrefStructureInit(const
AutoVarEmission &emission) {
auto layoutInfo = CGM.getObjCRuntime().BuildByrefLayout(CGM, type);
storeHeaderField(layoutInfo, getPointerSize(), "byref.layout");
}
+
+ if (emission.NeedsInitOnHeap)
+ emitByrefInitOnHeap(pointer);
----------------
ille-apple wrote:
Argh, you're right.
In principle the `BLOCK_HAS_COPY_DISPOSE` flag could be used for that purpose,
instead of creating a separate field in the payload. It could start out unset
(making the runtime skip the call to `byref_dispose`), and then be set after
initialization.
But the initialization might send the block capturing the object to another
thread. If initialization then throws, I assume that nobody will subsequently
_call_ the block, or if they do, the block won't actually try to use the
object. That's straightforward UB. But it doesn't seem like it should be UB
just to send the block. So there might be concurrent calls to
`_Block_object_dispose`. Not only that, 'our' `_Block_object_dispose` call
(the one that would hypothetically be added on the cleanup path to fix the
memory leak) might not be the one to drop the last reference.
Therefore, `flags` would have to be modified atomically, particularly because
the same word is also used for the reference count. And even with an atomic
operation, you would still have theoretical UB in the blocks runtime when it
reads `flags` non-atomically. Though, not any more UB than it's already
incurring thanks to potential concurrent refcount modifications... But what if
an alternate block runtime tries to cache `flags`? The only alternate runtime
I know of is GNUstep's and it looks like [it doesn't cache
it](https://github.com/gnustep/libobjc2/blob/master/blocks_runtime.m#L199), but
this still feels very dubious.
So then there's the alternative you suggested of just acknowledging that this
isn't exception-safe (specifically, that it leaks memory if initialization
throws). But that makes this whole patch feel like an awkward middle ground.
If self-capturing `__block` variables are not going to be supported properly
(pending new APIs), then maybe they shouldn't be supported at all and should
just be an error (pending new APIs).
But if new APIs are added, teaching Clang how to recognize when the target OS
supports them would be quite a lot of work for what is ultimately an edge case.
I'm going to see how ugly it ends up being to add a "successfully initialized"
flag.
https://github.com/llvm/llvm-project/pull/89475
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
