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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits