[PATCH] Permit OpenCL event_t's to be constructed from integer value 0.

[Richard Smith]

  Can you show me where in the OpenCL spec this language rule is described? I can't find any allowance for casting zero to `event_t`.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6788
@@ -6787,1 +6787,3 @@
   "variable in constant address space must be initialized">;
+def error_opencl_cast_non_zero_to_event_t : Error<
+  "cannot cast non-zero value '%0' to 'event_t'">;
----------------
`err_`, not `error_`.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6789
@@ -6788,1 +6788,3 @@
+def error_opencl_cast_non_zero_to_event_t : Error<
+  "cannot cast non-zero value '%0' to 'event_t'">;
 } // end of sema category
----------------
Maybe '%1' instead of 'event_t' here, so we get a better diagnostic if the target type is a typedef ("cannot cast non-zero value '1' to 'foo' (aka 'event_t')")?

================
Comment at: lib/Sema/SemaCast.cpp:2242
@@ +2241,3 @@
+    // for OpenCL, allow casts from '0' to event_t type
+    if ((Self.getLangOpts().OpenCL) && DestType->isEventT()) {
+      llvm::APSInt intValue;
----------------
Remove redundant parens around OpenCL check.

================
Comment at: lib/Sema/SemaCast.cpp:2243
@@ +2242,3 @@
+    if ((Self.getLangOpts().OpenCL) && DestType->isEventT()) {
+      llvm::APSInt intValue;
+      if(SrcExpr.get()->EvaluateAsInt(intValue, Self.getASTContext())) {
----------------
Should be `IntValue`

================
Comment at: lib/Sema/SemaCast.cpp:2241
@@ -2240,1 +2240,3 @@
 
+    // for OpenCL, allow casts from '0' to event_t type
+    if ((Self.getLangOpts().OpenCL) && DestType->isEventT()) {
----------------
Capital F in For.

================
Comment at: lib/Sema/SemaCast.cpp:2244-2245
@@ +2243,4 @@
+      llvm::APSInt intValue;
+      if(SrcExpr.get()->EvaluateAsInt(intValue, Self.getASTContext())) {
+        if(0 == intValue) {
+          return;
----------------
Space between `if` and `(`.

================
Comment at: lib/Sema/SemaCast.cpp:2245
@@ +2244,3 @@
+      if(SrcExpr.get()->EvaluateAsInt(intValue, Self.getASTContext())) {
+        if(0 == intValue) {
+          return;
----------------
We don't do the constant-on-the-left thing in the Clang codebase. Also, `operator!` is more efficient than `!= 0` for `APSInt`.

================
Comment at: lib/Sema/SemaCast.cpp:2251
@@ +2250,3 @@
+          SrcExpr = ExprError();
+          return;
+        }
----------------
Both arms of this `if/else` return, factor that out?

================
Comment at: lib/Sema/SemaCast.cpp:2244
@@ +2243,3 @@
+      llvm::APSInt intValue;
+      if(SrcExpr.get()->EvaluateAsInt(intValue, Self.getASTContext())) {
+        if(0 == intValue) {
----------------
Richard Smith wrote:
> Space between `if` and `(`.
`EvaluateAsInt` is inappropriate here. I'm not sure exactly what the language rule is, but this surely isn't it. Maybe take a look at `TryOCLZeroEventInitialization` in `SemaInit.cpp`.


http://llvm-reviews.chandlerc.com/D2860