r295150 - [Sema] Disallow returning a __block variable via a move.

[Hans Wennborg via cfe-commits]

+Richard for risk/reward analysis.

r274291 was also in 3.9, so this isn't strictly speaking a regression.

On Wed, Feb 15, 2017 at 11:43 AM, Akira Hatanaka <ahatanaka at apple.com> wrote:
> Hans,
>
> Can this be merged to 4.0 too?
>
>> On Feb 14, 2017, at 9:15 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: ahatanak
>> Date: Tue Feb 14 23:15:28 2017
>> New Revision: 295150
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=295150&view=rev
>> Log:
>> [Sema] Disallow returning a __block variable via a move.
>>
>> r274291 made changes to prefer calling a move constructor to calling a
>> copy constructor when returning from a function. This caused programs to
>> crash when a __block variable in the heap was moved out and used later.
>>
>> This commit fixes the bug by disallowing moving out of __block variables
>> implicitly.
>>
>> rdar://problem/28181080
>>
>> Differential Revision: https://reviews.llvm.org/D29908
>>
>> Modified:
>>    cfe/trunk/lib/Sema/SemaStmt.cpp
>>    cfe/trunk/test/SemaObjCXX/blocks.mm
>>
>> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=295150&r1=295149&r2=295150&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Feb 14 23:15:28 2017
>> @@ -2743,15 +2743,17 @@ bool Sema::isCopyElisionCandidate(QualTy
>>   // ...automatic...
>>   if (!VD->hasLocalStorage()) return false;
>>
>> +  // Return false if VD is a __block variable. We don't want to implicitly move
>> +  // out of a __block variable during a return because we cannot assume the
>> +  // variable will no longer be used.
>> +  if (VD->hasAttr<BlocksAttr>()) return false;
>> +
>>   if (AllowParamOrMoveConstructible)
>>     return true;
>>
>>   // ...non-volatile...
>>   if (VD->getType().isVolatileQualified()) return false;
>>
>> -  // __block variables can't be allocated in a way that permits NRVO.
>> -  if (VD->hasAttr<BlocksAttr>()) return false;
>> -
>>   // Variables with higher required alignment than their type's ABI
>>   // alignment cannot use NRVO.
>>   if (!VD->getType()->isDependentType() && VD->hasAttr<AlignedAttr>() &&
>>
>> Modified: cfe/trunk/test/SemaObjCXX/blocks.mm
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/blocks.mm?rev=295150&r1=295149&r2=295150&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjCXX/blocks.mm (original)
>> +++ cfe/trunk/test/SemaObjCXX/blocks.mm Tue Feb 14 23:15:28 2017
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
>> +// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++11 %s
>> @protocol NSObject;
>>
>> void bar(id(^)(void));
>> @@ -144,3 +144,17 @@ namespace DependentReturn {
>>
>>   template void f<X>(X);
>> }
>> +
>> +namespace MoveBlockVariable {
>> +struct B0 {
>> +};
>> +
>> +struct B1 { // expected-note 2 {{candidate constructor (the implicit}}
>> +  B1(B0&&); // expected-note {{candidate constructor not viable}}
>> +};
>> +
>> +B1 test_move() {
>> +  __block B0 b;
>> +  return b; // expected-error {{no viable conversion from returned value of type 'MoveBlockVariable::B0' to function return type 'MoveBlockVariable::B1'}}
>> +}
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>