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