rjmccall added a comment. In https://reviews.llvm.org/D49718#1174281, @ahatanak wrote:
> Merge pushBlockObjectRelease and enterByrefCleanup. > > In https://reviews.llvm.org/D49718#1174113, @rjmccall wrote: > > > 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. > > > I tried passing `BLOCK_FIELD_IS_WEAK | BLOCK_FIELD_IS_BYREF` to the call to > enterByrefCleanup in EmitAutoVarCleanups, but it looks like > test/CodeGenObjC/blocks.m fails if I do so. The test was committed in r125823 > and there is a comment that says " We're not supposed to pass > BLOCK_FIELD_IS_WEAK here". > > http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20110214/038996.html > > Should we change the check in the test? Thanks for the archaeology; that's a very good question. It looks to me like that comment was unrelated to the functionality change made by that patch, and nothing in the radar history explains it, either. I don't see anything in the runtime source which suggests that it's wrong to pass `BYREF|WEAK` to `Block_object_dispose`. Going way back, it does seem to have been necessary under GC to pass `BYREF|WEAK` for `Block_object_assign`, but the runtime never did anything special with the flag for `Block_object_dispose`; it just always treated it exactly the same as `BYREF` alone. I think it's fine to just change the test. Repository: rC Clang https://reviews.llvm.org/D49718 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
