[PATCH] Error on illegal OpenCL kernel argument types

[Richard Smith]

  For the struct diagnostic, it's not very clear what the notes mean (sure, there are members declared there, but why are you pointing them out?). Maybe you could produce something like:

      test/SemaOpenCL/arst2.cl:35:62: error: 'NestedPointer' cannot be used as the type of a kernel function parameter because it contains a field of pointer type
              kernel void pointer_in_nested_struct_arg(struct NestedPointer arg, struct NestedPointer second) { }// expected-error{{struct or union kernel parameters may not contain OpenCL objects}}
                                                                            ^
      test/SemaOpenCL/arst2.cl:32:16: note: within field of type 'Inner' declared here
                struct Inner inner;
                             ^
      test/SemaOpenCL/arst2.cl:26:21: note: within field of type 'InnerInner' declared here
                struct InnerInner b;
                                  ^
      test/SemaOpenCL/arst2.cl:12:8: note: field of pointer type 'int*' declared here
                int* foo;
                     ^


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6453
@@ +6452,3 @@
+def err_bad_kernel_param_type : Error<
+  "%0 cannot be used to declare a kernel function parameter">;
+def err_struct_with_pointers_kernel_param : Error<
----------------
"%0 cannot be used as the type of a kernel function parameter" would be better.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6454-6455
@@ -6454,1 +6453,4 @@
+  "%0 cannot be used to declare a kernel function parameter">;
+def err_struct_with_pointers_kernel_param : Error<
+  "struct or union kernel parameters may not contain OpenCL objects">;
 def err_event_t_global_var : Error<
----------------
Instead of "struct or union", please say which, and instead of "OpenCL objects" (which is not correct in several cases) please include the actual type as you do in the previous diagnostic.

================
Comment at: include/clang/Sema/Sema.h:1449-1450
@@ -1448,2 +1448,4 @@
   void ActOnEndFunctionDeclarator();
+
+  void checkIsValidOpenCLKernelArgument(Declarator &D, ParmVarDecl *P);
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
----------------
Remove this, you don't define such a Sema member any more.

================
Comment at: lib/Sema/SemaDecl.cpp:6178-6196
@@ +6177,21 @@
+  do {
+    const Decl *Next = VisitStack.pop_back_val();
+    if (!Next) {
+      // Found a marker, we have gone up a level
+      HistoryStack.pop_back();
+      continue;
+    }
+
+    // Adds everything except the original parameter declaration (which is not a
+    // field itself) to the history stack.
+    const RecordDecl *RD;
+    if (const FieldDecl *Field = dyn_cast<FieldDecl>(Next)) {
+      HistoryStack.push_back(Field);
+      RD = Field->getType()->castAs<RecordType>()->getDecl();
+    } else {
+      RD = cast<RecordDecl>(Next);
+    }
+
+    // Add a null marker so we know when we've gone back up a level
+    VisitStack.push_back((const Decl *) 0);
+
----------------
This would be cleaner if you made this function recursive.

================
Comment at: lib/Sema/SemaDecl.cpp:6236
@@ +6235,3 @@
+        << FD->getDeclName();
+      D.setInvalidType();
+    }
----------------
I don't think this is necessary.


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