[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
This revision was automatically updated to reflect the committed changes. Closed by commit rL299646: Fix lambda to block conversion in C++17 by avoiding copy elision for the (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D31669?vs=94177=94358#toc Repository: rL LLVM https://reviews.llvm.org/D31669 Files: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Initialization.h cfe/trunk/lib/Sema/SemaInit.cpp cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/CodeGenObjCXX/lambda-to-block.mm Index: cfe/trunk/include/clang/Sema/Initialization.h === --- cfe/trunk/include/clang/Sema/Initialization.h +++ cfe/trunk/include/clang/Sema/Initialization.h @@ -70,6 +70,9 @@ /// \brief The entity being initialized is a field of block descriptor for /// the copied-in c++ object. EK_BlockElement, +/// The entity being initialized is a field of block descriptor for the +/// copied-in lambda object that's used in the lambda to block conversion. +EK_LambdaToBlockConversionBlockElement, /// \brief The entity being initialized is the real or imaginary part of a /// complex number. EK_ComplexElement, @@ -260,7 +263,13 @@ QualType Type, bool NRVO) { return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO); } - + + static InitializedEntity InitializeLambdaToBlock(SourceLocation BlockVarLoc, + QualType Type, bool NRVO) { +return InitializedEntity(EK_LambdaToBlockConversionBlockElement, + BlockVarLoc, Type, NRVO); + } + /// \brief Create the initialization entity for an exception object. static InitializedEntity InitializeException(SourceLocation ThrowLoc, QualType Type, bool NRVO) { Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -1689,8 +1689,8 @@ "cannot initialize %select{a variable|a parameter|return object|an " "exception object|a member subobject|an array element|a new value|a value|a " "base class|a constructor delegation|a vector element|a block element|a " - "complex element|a lambda capture|a compound literal initializer|a " - "related result|a parameter of CF audited function}0 " + "block element|a complex element|a lambda capture|a compound literal " + "initializer|a related result|a parameter of CF audited function}0 " "%diff{of type $ with an %select{rvalue|lvalue}2 of type $|" "with an %select{rvalue|lvalue}2 of incompatible type}1,3" "%select{|: different classes%diff{ ($ vs $)|}5,6" Index: cfe/trunk/test/CodeGenObjCXX/lambda-to-block.mm === --- cfe/trunk/test/CodeGenObjCXX/lambda-to-block.mm +++ cfe/trunk/test/CodeGenObjCXX/lambda-to-block.mm @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -x objective-c++ -fblocks -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 -std=c++1z -emit-llvm -o - %s | FileCheck %s + +// rdar://31385153 +// Shouldn't crash! + +void takesBlock(void (^)(void)); + +struct Copyable { + Copyable(const Copyable ); +}; + +void hasLambda(Copyable x) { + takesBlock([x] () { }); +} +// CHECK-LABEL: define internal void @__copy_helper_block_ +// CHECK: call void @"_ZZ9hasLambda8CopyableEN3$_0C1ERKS0_" +// CHECK-LABEL: define internal void @"_ZZ9hasLambda8CopyableEN3$_0C2ERKS0_" +// CHECK: call void @_ZN8CopyableC1ERKS_ Index: cfe/trunk/lib/Sema/SemaLambda.cpp === --- cfe/trunk/lib/Sema/SemaLambda.cpp +++ cfe/trunk/lib/Sema/SemaLambda.cpp @@ -1649,10 +1649,9 @@ CallOperator->markUsed(Context); ExprResult Init = PerformCopyInitialization( - InitializedEntity::InitializeBlock(ConvLocation, - Src->getType(), - /*NRVO=*/false), - CurrentLocation, Src); + InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType(), + /*NRVO=*/false), + CurrentLocation, Src); if (!Init.isInvalid()) Init = ActOnFinishFullExpr(Init.get()); Index: cfe/trunk/lib/Sema/SemaInit.cpp === --- cfe/trunk/lib/Sema/SemaInit.cpp +++ cfe/trunk/lib/Sema/SemaInit.cpp @@ -950,6 +950,7 @@ case InitializedEntity::EK_Base: case InitializedEntity::EK_Delegating: case InitializedEntity::EK_BlockElement: + case InitializedEntity::EK_LambdaToBlockConversionBlockElement: case InitializedEntity::EK_Binding:
[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
rjmccall added a comment. Looks good, thanks. Repository: rL LLVM https://reviews.llvm.org/D31669 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
arphaman updated this revision to Diff 94177. arphaman marked 2 inline comments as done. arphaman added a comment. Use a new EntityKind and improve test. Repository: rL LLVM https://reviews.llvm.org/D31669 Files: include/clang/Sema/Initialization.h lib/Sema/SemaInit.cpp lib/Sema/SemaLambda.cpp test/CodeGenObjCXX/lambda-to-block.mm Index: test/CodeGenObjCXX/lambda-to-block.mm === --- /dev/null +++ test/CodeGenObjCXX/lambda-to-block.mm @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -x objective-c++ -fblocks -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 -std=c++1z -emit-llvm -o - %s | FileCheck %s + +// rdar://31385153 +// Shouldn't crash! + +void takesBlock(void (^)(void)); + +struct Copyable { + Copyable(const Copyable ); +}; + +void hasLambda(Copyable x) { + takesBlock([x] () { }); +} +// CHECK-LABEL: define internal void @__copy_helper_block_ +// CHECK: call void @"_ZZ9hasLambda8CopyableEN3$_0C1ERKS0_" +// CHECK-LABEL: define internal void @"_ZZ9hasLambda8CopyableEN3$_0C2ERKS0_" +// CHECK: call void @_ZN8CopyableC1ERKS_ Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -1649,10 +1649,9 @@ CallOperator->markUsed(Context); ExprResult Init = PerformCopyInitialization( - InitializedEntity::InitializeBlock(ConvLocation, - Src->getType(), - /*NRVO=*/false), - CurrentLocation, Src); + InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType(), + /*NRVO=*/false), + CurrentLocation, Src); if (!Init.isInvalid()) Init = ActOnFinishFullExpr(Init.get()); Index: lib/Sema/SemaInit.cpp === --- lib/Sema/SemaInit.cpp +++ lib/Sema/SemaInit.cpp @@ -950,6 +950,7 @@ case InitializedEntity::EK_Base: case InitializedEntity::EK_Delegating: case InitializedEntity::EK_BlockElement: + case InitializedEntity::EK_LambdaToBlockConversionBlockElement: case InitializedEntity::EK_Binding: llvm_unreachable("unexpected braced scalar init"); } @@ -2934,6 +2935,7 @@ case EK_VectorElement: case EK_ComplexElement: case EK_BlockElement: + case EK_LambdaToBlockConversionBlockElement: case EK_CompoundLiteralInit: case EK_RelatedResult: return DeclarationName(); @@ -2963,6 +2965,7 @@ case EK_VectorElement: case EK_ComplexElement: case EK_BlockElement: + case EK_LambdaToBlockConversionBlockElement: case EK_LambdaCapture: case EK_CompoundLiteralInit: case EK_RelatedResult: @@ -2992,6 +2995,7 @@ case EK_VectorElement: case EK_ComplexElement: case EK_BlockElement: + case EK_LambdaToBlockConversionBlockElement: case EK_LambdaCapture: case EK_RelatedResult: break; @@ -3025,6 +3029,9 @@ case EK_VectorElement: OS << "VectorElement " << Index; break; case EK_ComplexElement: OS << "ComplexElement " << Index; break; case EK_BlockElement: OS << "Block"; break; + case EK_LambdaToBlockConversionBlockElement: +OS << "Block (lambda)"; +break; case EK_LambdaCapture: OS << "LambdaCapture "; OS << DeclarationName(Capture.VarID); @@ -3622,9 +3629,13 @@ // destination object. // Per DR (no number yet), this does not apply when initializing a base // class or delegating to another constructor from a mem-initializer. + // ObjC++: Lambda captured by the block in the lambda to block conversion + // should avoid copy elision. if (S.getLangOpts().CPlusPlus1z && Entity.getKind() != InitializedEntity::EK_Base && Entity.getKind() != InitializedEntity::EK_Delegating && + Entity.getKind() != + InitializedEntity::EK_LambdaToBlockConversionBlockElement && UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isRValue() && S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) { // Convert qualifications if necessary. @@ -5488,6 +5499,7 @@ case InitializedEntity::EK_VectorElement: case InitializedEntity::EK_ComplexElement: case InitializedEntity::EK_BlockElement: + case InitializedEntity::EK_LambdaToBlockConversionBlockElement: case InitializedEntity::EK_LambdaCapture: case InitializedEntity::EK_CompoundLiteralInit: return Sema::AA_Initializing; @@ -5511,6 +5523,7 @@ case InitializedEntity::EK_ComplexElement: case InitializedEntity::EK_Exception: case InitializedEntity::EK_BlockElement: + case InitializedEntity::EK_LambdaToBlockConversionBlockElement: case InitializedEntity::EK_LambdaCapture: case InitializedEntity::EK_CompoundLiteralInit: return false; @@ -5537,6 +5550,7 @@ case InitializedEntity::EK_VectorElement: case
[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
rjmccall added inline comments. Comment at: include/clang/Sema/Initialization.h:124 +/// is a lambda that's captured by a block it's converted to. +bool IsLambdaToBlockConversionEntity; }; It seems less invasive to just give this a new EntityKind. Repository: rL LLVM https://reviews.llvm.org/D31669 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
aprantl added inline comments. Comment at: include/clang/Sema/Initialization.h:122 + +/// \brief When Kind == EK_BlockElement, this flag determines if the entity +/// is a lambda that's captured by a block it's converted to. No need to use \brief any more. LLVM uses autobrief. Repository: rL LLVM https://reviews.llvm.org/D31669 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block
arphaman created this revision. The commit r288866 introduced guaranteed copy elision to C++ 17. This unfortunately broke the lambda to block conversion in C++17 (the compiler crashes when performing IRGen). This patch fixes the conversion by avoiding copy elision for the capture that captures the lambda that's used in the block created by lambda to block conversion process. Repository: rL LLVM https://reviews.llvm.org/D31669 Files: include/clang/Sema/Initialization.h lib/Sema/SemaInit.cpp lib/Sema/SemaLambda.cpp test/CodeGenObjCXX/lambda-to-block.mm Index: test/CodeGenObjCXX/lambda-to-block.mm === --- /dev/null +++ test/CodeGenObjCXX/lambda-to-block.mm @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -x objective-c++ -fblocks -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 -std=c++1z -emit-llvm -o - %s | FileCheck %s + +// rdar://31385153 +// Shouldn't crash! + +void takesBlock(void (^)(void)); + +struct Copyable { + Copyable(const Copyable ); +}; + +void hasLambda(Copyable x) { + takesBlock([x] () { }); +} +// CHECK-LABEL: __copy_helper_block_ +// CHECK: call void @_ZN8CopyableC1ERKS_ +// CHECK-LABE: __destroy_helper_block_ Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -1649,10 +1649,10 @@ CallOperator->markUsed(Context); ExprResult Init = PerformCopyInitialization( - InitializedEntity::InitializeBlock(ConvLocation, - Src->getType(), - /*NRVO=*/false), - CurrentLocation, Src); + InitializedEntity::InitializeBlock(ConvLocation, Src->getType(), + /*NRVO=*/false, + /*IsLambdaToBlockConversion=*/true), + CurrentLocation, Src); if (!Init.isInvalid()) Init = ActOnFinishFullExpr(Init.get()); Index: lib/Sema/SemaInit.cpp === --- lib/Sema/SemaInit.cpp +++ lib/Sema/SemaInit.cpp @@ -3000,6 +3000,10 @@ return false; } +bool InitializedEntity::isLambdaToBlockEntity() const { + return Kind == EK_BlockElement && LocAndNRVO.IsLambdaToBlockConversionEntity; +} + unsigned InitializedEntity::dumpImpl(raw_ostream ) const { assert(getParent() != this); unsigned Depth = getParent() ? getParent()->dumpImpl(OS) : 0; @@ -3625,7 +3629,8 @@ if (S.getLangOpts().CPlusPlus1z && Entity.getKind() != InitializedEntity::EK_Base && Entity.getKind() != InitializedEntity::EK_Delegating && - UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isRValue() && + !Entity.isLambdaToBlockEntity() && UnwrappedArgs.size() == 1 && + UnwrappedArgs[0]->isRValue() && S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) { // Convert qualifications if necessary. Sequence.AddQualificationConversionStep(DestType, VK_RValue); Index: include/clang/Sema/Initialization.h === --- include/clang/Sema/Initialization.h +++ include/clang/Sema/Initialization.h @@ -118,6 +118,10 @@ /// \brief Whether the entity being initialized may end up using the /// named return value optimization (NRVO). bool NRVO; + +/// \brief When Kind == EK_BlockElement, this flag determines if the entity +/// is a lambda that's captured by a block it's converted to. +bool IsLambdaToBlockConversionEntity; }; struct VD { @@ -180,11 +184,11 @@ /// function, throwing an object, performing an explicit cast, or /// initializing a parameter for which there is no declaration. InitializedEntity(EntityKind Kind, SourceLocation Loc, QualType Type, -bool NRVO = false) -: Kind(Kind), Parent(nullptr), Type(Type), ManglingNumber(0) - { +bool NRVO = false, bool IsLambdaToBlockConversion = false) + : Kind(Kind), Parent(nullptr), Type(Type), ManglingNumber(0) { LocAndNRVO.Location = Loc.getRawEncoding(); LocAndNRVO.NRVO = NRVO; +LocAndNRVO.IsLambdaToBlockConversionEntity = IsLambdaToBlockConversion; } /// \brief Create the initialization entity for a member subobject. @@ -256,9 +260,11 @@ return InitializedEntity(EK_Result, ReturnLoc, Type, NRVO); } - static InitializedEntity InitializeBlock(SourceLocation BlockVarLoc, - QualType Type, bool NRVO) { -return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO); + static InitializedEntity + InitializeBlock(SourceLocation BlockVarLoc, QualType Type, bool NRVO, + bool IsLambdaToBlockConversion = false) { +return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO, +