[PATCH] D134445: [PR57881][OpenCL] Fix incorrect diagnostics with templated types in kernel arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D134445#3920257 <https://reviews.llvm.org/D134445#3920257>, @Anastasia wrote:

> In D134445#3920188 <https://reviews.llvm.org/D134445#3920188>, @rjmccall wrote:
>
>> You really can't ask whether a class template pattern is standard layout; it's not meaningful.
>
> Well the templates have to be instantiated with concrete types as kernels can't be called from the host code. But if the concrete type is a reference or pointer there is no full instantiation apparently as AST dumps in the review description show.

My objection here is to the code that does `CXXRec = CXXRec->getTemplateInstantiationPattern()`.  That is the unsubstituted template pattern.  You then ask if it has standard layout.  I do not think this is the right thing to do.

>> How are pointers and references passed to kernels?  Does the pointee get copied or something?  If so, you may have a requirement that pointee types be complete, in which case the only problem is probably that you're doing this check too soon, or doing them on declarations rather than on definition/use.
>
> Ok, the kernel function actually already contains the instantiated templates since they can't be templated themselves.

Yes, I understand that.

> My understanding is that references and pointers to templated types are not required to be fully instantiated as they are not ODR-used?

It's not the ODR, but yes, in standard C++, declaring a parameter of pointer type does not require the pointee type to be complete, and if the pointee type is a template type, the compiler is not permitted to immediately try to instantiate it.  Of course, OpenCL `kernel` declarations are not standard C++ and can use different rules, and I think the fact that OpenCL has this semantic restriction on pointer arguments to kernels is pretty good license to do so.  There are some places in C++ (e.g. initialization) that do instantiate templates if a definition is available, and you can request this by calling `Sema::isCompleteType` instead of `hasDefinition()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134445/new/

https://reviews.llvm.org/D134445