[PATCH] D31669: Fix lambda to block conversion in C++17 by avoiding copy elision for the lambda capture used by the created block

2017-04-06 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-04-05 Thread John McCall via Phabricator via cfe-commits
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

2017-04-05 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-04-04 Thread John McCall via Phabricator via cfe-commits
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

2017-04-04 Thread Adrian Prantl via Phabricator via cfe-commits
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

2017-04-04 Thread Alex Lorenz via Phabricator via cfe-commits
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,
+