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. https://reviews.llvm.org/D29908 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
