ille added a comment.
One downside of that approach is performance. It's somewhat idiosyncratic, but
I work on codebases that use blocks heavily in performance-sensitive paths.
Typically, the blocks are never copied (they don't escape) and are expected to
be inlined. Any implicit heap allocations added by the compiler would have an
undesirable performance cost, and would likely also hinder inlining. For those
codebases, I would much prefer to be prompted to refactor the code to avoid the
self-capture. I suppose this could be implemented as a warning, perhaps off by
default.
Another, possibly more serious issue is allocation failure. `Block_copy`
returns null rather than aborting on allocation failure. This is from the
Darwin userland implementation:
// Its a stack block. Make a copy.
if (!isGC) {
struct Block_layout *result = malloc(aBlock->descriptor->size);
if (!result) return NULL;
xnu also has a `Block_copy` implementation that can fail.
If Clang implicitly called `Block_copy` and it failed, it would have to either
hand the user a null block, which would be unexpected, or abort the process.
For most codebases, aborting the process would be fine, but there are codebases
that attempt to recover cleanly from all allocation failures, and some of those
may happen to use blocks. xnu is one example; I don't know of any others, but
they might exist.
I suppose those codebases could also enable the hypothetical warning, but it
seems like a footgun, at least if the warning is off by default.
Incidentally, I checked how this behaves under ARC. ARC has to deal with the
same situation, of course, but Objective-C generally aborts on allocation
failure, so having failed block copies do the same would be more acceptable and
is the behavior I'd expect. However, `objc_retainBlock`, the function ARC
generates calls to, simply forwards directly to `Block_copy`; it doesn't check
for a null return value, nor does the ARC-generated code do so itself. So the
user just gets a null block, which seems like a bug.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89903/new/
https://reviews.llvm.org/D89903
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
