[PATCH] D17436: [OpenCL] Add Sema checks for OpenCL 2.0 block

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7719
@@ +7718,3 @@
+def err_opencl_block_proto_variadic : Error<
+  "invalid block prototype, variadic arguments are not allowed in opencl">;
+def err_opencl_invalid_block_array : Error<
----------------
opencl -> OpenCL

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7723
@@ -7716,1 +7722,3 @@
+def err_opencl_ternary_with_block : Error<
+  "blocks cannot be used as expressions in ternary expressions in opencl">;
 
----------------
opencl -> OpenCL

================
Comment at: lib/Sema/SemaDecl.cpp:6712
@@ +6711,3 @@
+
+  // OpenCL v2.0 s6.12.5 - The following Blocks features are currently not
+  // supported in OpenCL C: Blocks with variadic arguments.
----------------
Could you write it a bit shorter, may be: Blocks with variadic arguments are not allowed.

================
Comment at: lib/Sema/SemaDecl.cpp:6714
@@ +6713,3 @@
+  // supported in OpenCL C: Blocks with variadic arguments.
+  if (getLangOpts().OpenCL && LangOpts.OpenCLVersion >= 200 &&
+      T->isBlockPointerType()) {
----------------
getLangOpts() -> LangOpts

Could you remove 'LangOpts.OpenCLVersion >= 200' because we can also enable Blocks with -fblocks flag.

================
Comment at: lib/Sema/SemaDecl.cpp:6717
@@ +6716,3 @@
+    const BlockPointerType *BlkTy = T->getAs<BlockPointerType>();
+    assert(BlkTy && "Not a block pointer.");
+
----------------
This seems redundant with T->isBlockPointerType() from above

================
Comment at: lib/Sema/SemaExpr.cpp:6520
@@ -6507,1 +6519,3 @@
 
+  // OpenCL v2.0 s6.12.5 - To support these behaviors, additional
+  // restrictions in addition to the above feature restrictions are: Blocks
----------------
Can you shorten this comment too. I would just remove:  "To support these behaviors, additional restrictions in addition to the above feature restrictions are:".

================
Comment at: lib/Sema/SemaExpr.cpp:6523
@@ +6522,3 @@
+  // cannot be used as expressions of the ternary selection operator (?:).
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) {
+    // should output error for both LHS and RHS, use | instead ||
----------------
Also could you remove 'getLangOpts().OpenCLVersion >= 200', see similar comment above.

================
Comment at: lib/Sema/SemaExpr.cpp:6525
@@ +6524,3 @@
+    // should output error for both LHS and RHS, use | instead ||
+    if (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get()))
+      return QualType();
----------------
I suggest to unify with another check for OpenCL on line 6497 to have OpenCL bits all in one place.

================
Comment at: lib/Sema/SemaExpr.cpp:10358
@@ +10357,3 @@
+  // OpenCL v2.0 s6.12.5 - The unary operators (* and &) cannot be used with a
+  // Block
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) {
----------------
Block -> block

================
Comment at: lib/Sema/SemaExpr.cpp:10359
@@ +10358,3 @@
+  // Block
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) {
+    if (OrigOp.get()->getType()->isBlockPointerType()) {
----------------
Could you remove 'getLangOpts().OpenCLVersion >= 200' here too!

================
Comment at: lib/Sema/SemaExpr.cpp:10411
@@ +10410,3 @@
+    // OpenCL v2.0 s6.12.5 - The unary operators (* and &) cannot be used with a
+    // Block.
+    if (S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200 &&
----------------
Block -> block

================
Comment at: lib/Sema/SemaType.cpp:2178
@@ -2177,1 +2177,3 @@
 
+  // OpenCL v2.0 s6.12.5 - The following Blocks features are currently not
+  // supported in OpenCL C: Arrays of Blocks.
----------------
Could you change to "Arrays of blocks are not supported.".



================
Comment at: test/SemaOpenCL/invalid-block.cl:11
@@ +10,3 @@
+  BlkInt B2 = ^int(int I) {return 2;};
+  BlkInt Arr[] = {B1, B2}; // expected-error {{array of block is invalid in OpenCL}}
+  int tmp = i ? B1(i)      // expected-error {{blocks cannot be used as expressions in ternary expressions}}
----------------
array of block type ...

================
Comment at: test/SemaOpenCL/invalid-block.cl:12
@@ +11,3 @@
+  BlkInt Arr[] = {B1, B2}; // expected-error {{array of block is invalid in OpenCL}}
+  int tmp = i ? B1(i)      // expected-error {{blocks cannot be used as expressions in ternary expressions}}
+              : B2(i);     // expected-error {{blocks cannot be used as expressions in ternary expressions}}
----------------
block type cannot ...

================
Comment at: test/SemaOpenCL/invalid-block.cl:13
@@ +12,3 @@
+  int tmp = i ? B1(i)      // expected-error {{blocks cannot be used as expressions in ternary expressions}}
+              : B2(i);     // expected-error {{blocks cannot be used as expressions in ternary expressions}}
+}
----------------
block type cannot ...

================
Comment at: test/SemaOpenCL/invalid-block.cl:16
@@ +15,3 @@
+
+void f2(BlkInt *BlockPtr) {
+  BlkInt B = ^int(int I) {return 1;};
----------------
Actually I think pointers to blocks have to be disallowed too, but spec doesn't mention that although it forbids the dereferencing.

I will submit a bug to Khronos to clarify this.


http://reviews.llvm.org/D17436