[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

Thu Mar 14 10:21:28 PDT 2019

wuhao5 added a comment.

Hey guys, this is Hao, I am working with Chrome team to sort this issue out.

In D58514#1428606 <https://reviews.llvm.org/D58514#1428606>, @rjmccall wrote:

> I remember this coming up 7-8 years ago.  I intentionally decided against doing a copy/release when assigning to `__weak` because if the block wasn't already guaranteed to be copied then it was probably better to crash than to silently assign a value that's about to be deallocated.  Note that copying the block we assign into `wb` doesn't change anything about the block stored in `b`.
>
> I don't know why Chromium is building a weak reference to a block in the first place, but assuming they have a good reason to be doing it, they should fix their code to force a copy before forming a weak reference.

The culprit code is here <https://github.com/google/eDistantObject/blob/0146bd6e1f057160bb80e596c644414e05267e9e/Service/Sources/EDOHostService.m#L295>

it's true that it can be copied before assigning to the weak var, and I understand the reasoning behind. however, my question is: just from the code itself, each variable has the proper scope and assignment, if the block copy happen automatically, just like what we should expect ARC would do, should it not mutate itself to something else. to be more precise, should the block assigned to the weak var be the same after the block is copied? (and in the code, the block should be moved to the heap after calling -addObject: a few line below.)

so in the end of day, as a user, should we expect the compiler would move the block from stack to heap in time and the variable we hold is consistent?

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58514/new/

https://reviews.llvm.org/D58514