rjmccall added a comment.

In https://reviews.llvm.org/D29908#676601, @ahatanak wrote:

> In https://reviews.llvm.org/D29908#676589, @ahatanak wrote:
> > Posting John's review comment, which was sent to the review I abandoned:
> >
> > "Hmm.  The behavior I think we actually want here is that we should move 
> > out of the __block variable but not perform NRVO, essentially like we would 
> > if the variable were a parameter.
> I think that's what's happening. Because of the changes made in r274291, NRVO 
> isn't performed, but the move constructor instead of the copy constructor is 
> called to return the shared_ptr. The shared_ptr in the heap is moved to 
> returned shared_ptr (%agg.result), so when the block function passed to 
> dispatch_async is executed, it segfaults because the object "ret" used to 
> point to has been destructed.
> If that's the case, should we conclude that clang is behaving as expected?
> > The block byref copy helper should always be doing a move if it can, 
> > regardless of whether the function is returned.  That's independent of 
> > this, though."

Oh, I see.  Just to clarify: it doesn't matter that the object "ret" used to 
point to has been destructed, what matters is that "ret" now holds a null 
reference because it's been moved out of.

I retract my comment; I agree there's a bug here.  We should not implicitly 
move out of __block variables during a return because we cannot assume that the 
variable will no longer be used.  Please update the comment to correctly 
identify this as the reasoning; it has nothing to do with whether __block 
variables technically *can* support NRVO.


cfe-commits mailing list

Reply via email to