[PATCH] D16040: [OpenCL] Refine OpenCLImageAccessAttr to OpenCLAccessAttr

[Anastasia Stulova via cfe-commits]

Anastasia added a comment.

I think some tests for new diagnostics are still missing?


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7676
@@ +7675,3 @@
+def err_opencl_invalid_read_write : Error<
+  "access qualifier read_write can not be used for %0 %select{|before CL2.0}1">;
+def err_opencl_multiple_access_qualifiers : Error<
----------------
before CL2.0 -> earlier than OpenCL2.0 versions

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4922
@@ +4921,3 @@
+
+  // OpenCL v2.0 s6.6: read_write can be used for image type The __read_write
+  // (or read_write)qualifier must be used with image object arguments of
----------------
could we write it shorter somehow:

read_write can be used for image types to specify that an image object can be read and written.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:5692
@@ +5691,3 @@
+    // attr pipe int p;
+    // which will process attr for twice, but we do not need to process that for
+    // twice, so skip this process if it is pipe type
----------------
Ah, you mean the attr was already process with element type?

I don't think this comment is clear still.

Could you write something like:
Skip if pipe type as already being processes with an element type.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:5696
@@ +5695,3 @@
+      if (PDecl->getType().getCanonicalType().getTypePtr()->isPipeType())
+        break;
+    }
----------------
Could you also avoid using break. I think it should be quite easy here.

================
Comment at: test/SemaOpenCL/invalid-kernel-attrs.cl:31-32
@@ -30,4 +30,4 @@
   int __kernel x; // expected-error {{'__kernel' attribute only applies to functions}}
-  read_only int i; // expected-error {{'read_only' attribute only applies to parameters}}
-  __write_only int j; // expected-error {{'__write_only' attribute only applies to parameters}}
+  read_only image1d_t i; // expected-error {{'read_only' attribute only applies to parameters}}
+  __write_only image2d_t j; // expected-error {{'__write_only' attribute only applies to parameters}}
 }
----------------
LG!


http://reviews.llvm.org/D16040