[PATCH] D58164: Block+lambda: allow reference capture
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe4dfc9f5bda3: Fix the type of the capture passed to
LambdaIntroducer::addCapture in… (authored by ahatanak).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58164/new/
https://reviews.llvm.org/D58164
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
Index: clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
===
--- clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
+++ clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++14 -fblocks -fobjc-arc -o - %s | FileCheck %s
// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+// CHECK: %[[S:.*]] = type { i32 }
+// CHECK: %[[CLASS_ANON_2:.*]] = type { %[[S]]* }
+// CHECK: %[[CLASS_ANON_3:.*]] = type { %[[S]] }
// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
@@ -33,7 +36,7 @@
// reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_3clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8\01?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
@@ -46,10 +49,60 @@
// is captured by reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_4clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8\01?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+void test1() {
+ id a = getObj(), b = getObj(), c = getObj();
+ [, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+struct S {
+ int val() const;
+ int a;
+ S();
+ S(const S&);
+ S =(const S&);
+ S(S&&);
+ S =(S&&);
+};
+
+S getS();
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test2EvENK3$_1clIiEEDaT_"(%[[CLASS_ANON_2]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_2]], %[[CLASS_ANON_2]]* %{{.*}}, i32 0, i32 0
+// CHECK: %[[V1:.*]] = load %[[S]]*, %[[S]]** %[[V0]], align 8
+// CHECK: store %[[S]]* %[[V1]], %[[S]]** %[[BLOCK_CAPTURED]], align 8
+
+int test2() {
+ S s;
+ auto fn = [&](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test3EvENK3$_2clIiEEDaT_"(%[[CLASS_ANON_3]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_3]], %[[CLASS_ANON_3]]* %{{.*}}, i32 0, i32 0
+// CHECK: call void @_ZN18CaptureByReference1SC1ERKS0_(%[[S]]* %[[BLOCK_CAPTURED]], %[[S]]* {{.*}} %[[V0]])
+
+int test3() {
+ const S = getS();
+ auto fn = [=](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
// CHECK-NOT: call void @llvm.objc.storeStrong(
// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0,
[PATCH] D58164: Block+lambda: allow reference capture
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, I think this is a reasonable fix. If @rsmith has thoughts about this, I guess they'll have to happen post-review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak updated this revision to Diff 248371.
ahatanak changed the repository for this revision from rC Clang to rG LLVM
Github Monorepo.
ahatanak added a comment.
Herald added a subscriber: ributzka.
Rebase and ping.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58164/new/
https://reviews.llvm.org/D58164
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
Index: clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
===
--- clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
+++ clang/test/CodeGenObjCXX/block-nested-in-lambda.mm
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++14 -fblocks -fobjc-arc -o - %s | FileCheck %s
// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+// CHECK: %[[S:.*]] = type { i32 }
+// CHECK: %[[CLASS_ANON_2:.*]] = type { %[[S]]* }
+// CHECK: %[[CLASS_ANON_3:.*]] = type { %[[S]] }
// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
@@ -33,7 +36,7 @@
// reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_3clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8\01?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
@@ -46,10 +49,60 @@
// is captured by reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_4clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8\01?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+void test1() {
+ id a = getObj(), b = getObj(), c = getObj();
+ [, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+struct S {
+ int val() const;
+ int a;
+ S();
+ S(const S&);
+ S =(const S&);
+ S(S&&);
+ S =(S&&);
+};
+
+S getS();
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test2EvENK3$_1clIiEEDaT_"(%[[CLASS_ANON_2]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_2]], %[[CLASS_ANON_2]]* %{{.*}}, i32 0, i32 0
+// CHECK: %[[V1:.*]] = load %[[S]]*, %[[S]]** %[[V0]], align 8
+// CHECK: store %[[S]]* %[[V1]], %[[S]]** %[[BLOCK_CAPTURED]], align 8
+
+int test2() {
+ S s;
+ auto fn = [&](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test3EvENK3$_2clIiEEDaT_"(%[[CLASS_ANON_3]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_3]], %[[CLASS_ANON_3]]* %{{.*}}, i32 0, i32 0
+// CHECK: call void @_ZN18CaptureByReference1SC1ERKS0_(%[[S]]* %[[BLOCK_CAPTURED]], %[[S]]* {{.*}} %[[V0]])
+
+int test3() {
+ const S = getS();
+ auto fn = [=](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
// CHECK-NOT: call void @llvm.objc.storeStrong(
// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8**
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak updated this revision to Diff 212016.
ahatanak added a comment.
Rebase.
I think the fix is correct. When the lambda expression for the generic lambda
is built, `BuildLambdaExpr` passes a `Capture` object in
`LambdaScopeInfo::Captures` to `BuildCaptureField` to build the closure class
field for the capture, so it should be okay to read the type of the field and
pass it to `LambdaScopeInfo::addCapture` when rebuilding the lambda scope info.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58164/new/
https://reviews.llvm.org/D58164
Files:
lib/Sema/SemaDecl.cpp
test/CodeGenObjCXX/block-nested-in-lambda.mm
Index: test/CodeGenObjCXX/block-nested-in-lambda.mm
===
--- test/CodeGenObjCXX/block-nested-in-lambda.mm
+++ test/CodeGenObjCXX/block-nested-in-lambda.mm
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++14 -fblocks -fobjc-arc -o - %s | FileCheck %s
// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+// CHECK: %[[S:.*]] = type { i32 }
+// CHECK: %[[CLASS_ANON_2:.*]] = type { %[[S]]* }
+// CHECK: %[[CLASS_ANON_3:.*]] = type { %[[S]] }
// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
@@ -33,7 +36,7 @@
// reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_3clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8\01?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
@@ -46,10 +49,60 @@
// is captured by reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_4clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8\01?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+void test1() {
+ id a = getObj(), b = getObj(), c = getObj();
+ [, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+struct S {
+ int val() const;
+ int a;
+ S();
+ S(const S&);
+ S =(const S&);
+ S(S&&);
+ S =(S&&);
+};
+
+S getS();
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test2EvENK3$_1clIiEEDaT_"(%[[CLASS_ANON_2]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_2]], %[[CLASS_ANON_2]]* %{{.*}}, i32 0, i32 0
+// CHECK: %[[V1:.*]] = load %[[S]]*, %[[S]]** %[[V0]], align 8
+// CHECK: store %[[S]]* %[[V1]], %[[S]]** %[[BLOCK_CAPTURED]], align 8
+
+int test2() {
+ S s;
+ auto fn = [&](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test3EvENK3$_2clIiEEDaT_"(%[[CLASS_ANON_3]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_3]], %[[CLASS_ANON_3]]* %{{.*}}, i32 0, i32 0
+// CHECK: call void @_ZN18CaptureByReference1SC1ERKS0_(%[[S]]* %[[BLOCK_CAPTURED]], %[[S]]* {{.*}} %[[V0]])
+
+int test3() {
+ const S = getS();
+ auto fn = [=](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
// CHECK-NOT: call void
[PATCH] D58164: Block+lambda: allow reference capture
rjmccall added a comment. Okay. In that case, yeah, it'd be good to get Richard's opinion about the right way to get that information here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak added a comment. Currently a block captures a variable (POD or non-POD) by reference if the enclosing lambda captures it by reference and captures by copy if the enclosing lambda captures by copy. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
rjmccall added a comment. I'm not sure we've ever written down what the semantics of the block capture are actually supposed to be in this situation. I guess capturing a reference is the right thing to do? Is that what we generally do if this is a POD type? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak added a reviewer: rsmith. ahatanak added a comment. Richard, could you shed light on why it's done this way? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak added a comment. I think I now have a better idea of what's causing the crash in IRGen. The root of the problem is that, when `RebuildLambdaScopeInfo` is called to rebuild the scope info for the generic lambda, the type of the captured variable (`s` in `test2` and `test3` in `test/CodeGenObjCXX/block-nested-in-lambda.mm`) is passed to `addCapture`, which is different from the type of the non-static member of the closure object. For example, the member type of the generic lambda in `test2` is `S&` whereas the type of variable `s` is `S`. `Sema::ActOnBlockStmtExpr` in SemaExpr.cpp uses the type passed to `addCapture` to determine whether copying from the lambda member to the field in the block structure requires a copy constructor, but since it isn't passed the correct type, it incorrectly determines that a copy constructor is needed when the capture is a by-reference capture (for example, in `test2`) and isn't needed when the capture is a by-copy capture (for example, in `test3`), which causes crashes in IRGen. I think the fix is to pass the correct capture type, which I think is whatever `I->getType()` returns, to `addCapture` in `RebuildLambdaScopeInfo`, but I'm wondering whether there is a reason `VD->getType()` has to be called here when the other two cases (this and VLA) uses `I->getType()`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak updated this revision to Diff 204882.
ahatanak added a comment.
- Add another test case which has a block nested inside a lambda and causes
clang to crash.
- Fix the capture type passed to `addCapture` in `RebuildLambdaScopeInfo`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58164/new/
https://reviews.llvm.org/D58164
Files:
lib/Sema/SemaDecl.cpp
test/CodeGenObjCXX/block-nested-in-lambda.mm
Index: test/CodeGenObjCXX/block-nested-in-lambda.mm
===
--- test/CodeGenObjCXX/block-nested-in-lambda.mm
+++ test/CodeGenObjCXX/block-nested-in-lambda.mm
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -fblocks -fobjc-arc -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -std=c++14 -fblocks -fobjc-arc -o - %s | FileCheck %s
// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 }
+// CHECK: %[[S:.*]] = type { i32 }
+// CHECK: %[[CLASS_ANON_2:.*]] = type { %[[S]]* }
+// CHECK: %[[CLASS_ANON_3:.*]] = type { %[[S]] }
// CHECK: %[[BLOCK_CAPTURED0:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>, <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i32*, i32* }>* %[[BLOCK:.*]], i32 0, i32 5
// CHECK: %[[V0:.*]] = getelementptr inbounds %[[LAMBDA_CLASS:.*]], %[[LAMBDA_CLASS]]* %[[THIS:.*]], i32 0, i32 0
@@ -33,7 +36,7 @@
// reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test0Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_1clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test0EvENK3$_3clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i64 }* @"__block_descriptor_40_e5_v8\01?0ls32l8" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
@@ -46,10 +49,60 @@
// is captured by reference.
// CHECK-LABEL: define void @_ZN18CaptureByReference5test1Ev(
-// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_2clEv"(
+// CHECK-LABEL: define internal void @"_ZZN18CaptureByReference5test1EvENK3$_4clEv"(
// CHECK: %[[BLOCK_DESCRIPTOR:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32 4
// CHECK: store %[[STRUCT_BLOCK_DESCRIPTOR]]* bitcast ({ i64, i64, i8*, i8*, i8*, i64 }* @"__block_descriptor_56_8_32s40s_e5_v8\01?0l" to %[[STRUCT_BLOCK_DESCRIPTOR]]*), %[[STRUCT_BLOCK_DESCRIPTOR]]** %[[BLOCK_DESCRIPTOR]], align 8
+void test1() {
+ id a = getObj(), b = getObj(), c = getObj();
+ [, b, c]{ ^{ a = 0; use(b); use(c); }(); }();
+}
+
+struct S {
+ int val() const;
+ int a;
+ S();
+ S(const S&);
+ S =(const S&);
+ S(S&&);
+ S =(S&&);
+};
+
+S getS();
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test2EvENK3$_1clIiEEDaT_"(%[[CLASS_ANON_2]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>, <{ i8*, i32, i32, i8*, %{{.*}}, %[[S]]* }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_2]], %[[CLASS_ANON_2]]* %{{.*}}, i32 0, i32 0
+// CHECK: %[[V1:.*]] = load %[[S]]*, %[[S]]** %[[V0]], align 8
+// CHECK: store %[[S]]* %[[V1]], %[[S]]** %[[BLOCK_CAPTURED]], align 8
+
+int test2() {
+ S s;
+ auto fn = [&](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
+// CHECK: define internal i32 @"_ZZN18CaptureByReference5test3EvENK3$_2clIiEEDaT_"(%[[CLASS_ANON_3]]* %{{.*}}, i32 %{{.*}})
+// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, align 8
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>, <{ i8*, i32, i32, i8*, %{{.*}}*, %[[S]] }>* %[[BLOCK]], i32 0, i32 5
+// CHECK: %[[V0:.*]] = getelementptr inbounds %[[CLASS_ANON_3]], %[[CLASS_ANON_3]]* %{{.*}}, i32 0, i32 0
+// CHECK: call void @_ZN18CaptureByReference1SC1ERKS0_(%[[S]]* %[[BLOCK_CAPTURED]], %[[S]]* {{.*}} %[[V0]])
+
+int test3() {
+ const S = getS();
+ auto fn = [=](const auto a){
+return ^{
+ return s.val();
+}();
+ };
+ return fn(123);
+}
+
// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_8_32s40s(
// CHECK-NOT: call void @llvm.objc.storeStrong(
// CHECK: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8*, i8** }>* %{{.*}}, i32 0, i32
[PATCH] D58164: Block+lambda: allow reference capture
rjmccall added a comment. I agree. There should just never be a copy expression if the capture is of reference type, and there should therefore be no need to special-case that in IRGen. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak requested changes to this revision. ahatanak added a comment. This revision now requires changes to proceed. Sorry, I didn't mean to accept this yet. I think this is something that is better fixed in Sema. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. Sorry, I was misunderstanding the problem. I was trying to understand why the crash goes away if I change the generic lambda to a non-generic one. What I found was that, when the lambda is generic, `captureInBlock` is passed a `CaptureType` that isn't an lvalue reference type and therefore performs copy initialization and sets the copy expression of the Capture to a non-null expression. This happens because `isVariableAlreadyCapturedInScopeInfo` doesn't assign the correct type (which should be an `LValueReferenceType`) to `CaptureType` when the variable is captured by reference. I think this can be fixed if we check whether `CaptureType` is a reference capture in `isVariableAlreadyCapturedInScopeInfo` and, if it is, turn it into an lvalue reference type. Comment at: test/CodeGenCXX/lambda-capturing-block.cpp:1 +// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -emit-llvm -std=c++17 -fblocks -fcxx-exceptions -o - %s | FileCheck %s + I think you can simplify the test case a bit more: - remove -fcxx-exceptions. - remove 'extern "C"'. - remove the derp's move constructor and destructor. - remove the try/catch and just call 'c.cancel' in the function. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
jfb added a comment. I talked to Akira in meatspace, and it seems like this updated patch does the right thing. He suggested changing the AST as a longer-term solution, but for now this approach seems simple enough. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
jfb updated this revision to Diff 186752.
jfb added a comment.
- Check for references when looking for copyexpr directly.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58164/new/
https://reviews.llvm.org/D58164
Files:
lib/CodeGen/CGBlocks.cpp
test/CodeGenCXX/lambda-capturing-block.cpp
Index: test/CodeGenCXX/lambda-capturing-block.cpp
===
--- /dev/null
+++ test/CodeGenCXX/lambda-capturing-block.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -emit-llvm -std=c++17
-fblocks -fcxx-exceptions -o - %s | FileCheck %s
+
+extern "C" {
+
+struct derp {
+derp() {}
+derp(const derp& _Src) {}
+derp(derp&& _Src) {}
+~derp() {}
+void cancel() const{}
+};
+
+// CHECK-LABEL: test(
+void test() {
+ derp c;
+ auto b = [&](auto const& func) noexcept {
+auto block = ^() {
+ try {
+func();
+ } catch (...) {
+c.cancel();
+ }
+};
+block();
+ };
+
+ b([](){});
+}
+
+}
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -622,7 +622,7 @@
}
// So do types that require non-trivial copy construction.
-} else if (CI.hasCopyExpr()) {
+} else if (CI.hasCopyExpr() && VT->getAsCXXRecordDecl()) {
info.NeedsCopyDispose = true;
info.HasCXXObject = true;
if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
Index: test/CodeGenCXX/lambda-capturing-block.cpp
===
--- /dev/null
+++ test/CodeGenCXX/lambda-capturing-block.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -emit-llvm -std=c++17 -fblocks -fcxx-exceptions -o - %s | FileCheck %s
+
+extern "C" {
+
+struct derp {
+derp() {}
+derp(const derp& _Src) {}
+derp(derp&& _Src) {}
+~derp() {}
+void cancel() const{}
+};
+
+// CHECK-LABEL: test(
+void test() {
+ derp c;
+ auto b = [&](auto const& func) noexcept {
+auto block = ^() {
+ try {
+func();
+ } catch (...) {
+c.cancel();
+ }
+};
+block();
+ };
+
+ b([](){});
+}
+
+}
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -622,7 +622,7 @@
}
// So do types that require non-trivial copy construction.
-} else if (CI.hasCopyExpr()) {
+} else if (CI.hasCopyExpr() && VT->getAsCXXRecordDecl()) {
info.NeedsCopyDispose = true;
info.HasCXXObject = true;
if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak added a comment. I think the root of the problem is that `BlockDecl` knows the captured variable but doesn't know the type of the capture. The type of the variable and the type of the capture can be different if the block is nested inside a lambda or another block, which is why `CI.hasCopyExpr()` can return a non-null value when `VT` is a lvalue reference: `-FunctionDecl 0x11306dc28 line:11:6 test 'void ()' |-DeclStmt 0x11306ddb0 | `-VarDecl 0x11306dd20 col:8 used c 'derp' callinit ... | |-CXXRecordDecl 0x11306dec0 col:12 implicit class definition ... | | |-FieldDecl 0x11309cc58 col:7 implicit 'derp &' ... | | | | `-BlockDecl 0x11309cb28 line:15:18 | | | | |-capture nested Var 0x11306dd20 'c' 'derp' Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
ahatanak added a comment. The code is crashing here because the loop in `computeBlockInfo` is trying to capture a variable that is captured by reference by the enclosing lambda as if it were captured by value. This is the type of VT: LValueReferenceType 0x1138008c0 'struct derp &' `-RecordType 0x113052580 'struct derp' `-CXXRecord 0x1130524e8 'derp' So in this case, the type doesn't require non-trivial copy construction. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58164: Block+lambda: allow reference capture
jfb created this revision.
jfb added reviewers: rjmccall, ahatanak.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.
Capturing a C++ object by reference wasn't quite working when mixing block and
lambda.
Repository:
rC Clang
https://reviews.llvm.org/D58164
Files:
lib/CodeGen/CGBlocks.cpp
test/CodeGenCXX/lambda-capturing-block.cpp
Index: test/CodeGenCXX/lambda-capturing-block.cpp
===
--- /dev/null
+++ test/CodeGenCXX/lambda-capturing-block.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -emit-llvm -std=c++17
-fblocks -fcxx-exceptions -o - %s | FileCheck %s
+
+extern "C" {
+
+struct derp {
+derp() {}
+derp(const derp& _Src) {}
+derp(derp&& _Src) {}
+~derp() {}
+void cancel() const{}
+};
+
+// CHECK-LABEL: test(
+void test() {
+ derp c;
+ auto b = [&](auto const& func) noexcept {
+auto block = ^() {
+ try {
+func();
+ } catch (...) {
+c.cancel();
+ }
+};
+block();
+ };
+
+ b([](){});
+}
+
+}
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -625,8 +625,13 @@
} else if (CI.hasCopyExpr()) {
info.NeedsCopyDispose = true;
info.HasCXXObject = true;
- if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
-info.CapturesNonExternalType = true;
+ if (auto *T = VT->getAsTagDecl()) {
+if (T->isExternallyVisible())
+ info.CapturesNonExternalType = true;
+ } else if (auto *P = VT->getPointeeCXXRecordDecl()) {
+if (P->isExternallyVisible())
+ info.CapturesNonExternalType = true;
+ }
// So do C structs that require non-trivial copy construction or
// destruction.
Index: test/CodeGenCXX/lambda-capturing-block.cpp
===
--- /dev/null
+++ test/CodeGenCXX/lambda-capturing-block.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -S -emit-llvm -std=c++17 -fblocks -fcxx-exceptions -o - %s | FileCheck %s
+
+extern "C" {
+
+struct derp {
+derp() {}
+derp(const derp& _Src) {}
+derp(derp&& _Src) {}
+~derp() {}
+void cancel() const{}
+};
+
+// CHECK-LABEL: test(
+void test() {
+ derp c;
+ auto b = [&](auto const& func) noexcept {
+auto block = ^() {
+ try {
+func();
+ } catch (...) {
+c.cancel();
+ }
+};
+block();
+ };
+
+ b([](){});
+}
+
+}
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -625,8 +625,13 @@
} else if (CI.hasCopyExpr()) {
info.NeedsCopyDispose = true;
info.HasCXXObject = true;
- if (!VT->getAsCXXRecordDecl()->isExternallyVisible())
-info.CapturesNonExternalType = true;
+ if (auto *T = VT->getAsTagDecl()) {
+if (T->isExternallyVisible())
+ info.CapturesNonExternalType = true;
+ } else if (auto *P = VT->getPointeeCXXRecordDecl()) {
+if (P->isExternallyVisible())
+ info.CapturesNonExternalType = true;
+ }
// So do C structs that require non-trivial copy construction or
// destruction.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
