[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

In D114080#3137480 <https://reviews.llvm.org/D114080#3137480>, @aaron.ballman wrote:

> In D114080#3137462 <https://reviews.llvm.org/D114080#3137462>, @Naghasan wrote:
>
>> right, TIL `sizeof (ContainsArr)` is 0 according to clang. I can see trouble arising.
>>
>> @aaron.ballman maybe you know better, is that intended for the extension ?
>
> I don't *know*, but I have two educated guesses that it is intended. GCC compatibility (GCC also has this behavior) and the relationship between zero-sized array members and flexible array members. Clang allows a flexible array member to be used in a structure with no members and it behaves the same as a zero-sized array in a structure with no members: https://godbolt.org/z/Gx7ozxYKz

I see. I was confused as it normally C++ requires at least 1 byte. But I guess you need that for compatibility with C.

In D114080#3137482 <https://reviews.llvm.org/D114080#3137482>, @Fznamznon wrote:

> In D114080#3137430 <https://reviews.llvm.org/D114080#3137430>, @Naghasan wrote:
>
>> Why the need for this restriction ?
>
> Zero length arrays are not supported by pure C++. It is an extension supported by clang. They are also not allowed in OpenCL and SPIR-V, and I even though I cannot grep an explicit restriction for them in SYCL 2020 spec (I guess that is just because it is an extension and not pure C++) It doesn't make sense to allow them here since memory allocation in device code is prohibited in SYCL by the spec. This diagnostic prevents further not always user friendly errors coming from SPIR-V emitters or from backends like OpenCL that explicitly disallow them on early stage and in user-friendly form. I believe other GPU programming models also may not allow them for the same reason - restriction on memory allocation.

SYCL won't mention this as it is an extension indeed. Same for VLA. SPIR-V is another question though, while 0 element array are not allowed, empty structs are (but IIRC size of an empty struct is not clear).

> It doesn't make sense to allow them here since memory allocation in device code is prohibited in SYCL by the spec

I agree, but having a field defined is different from it being used. It is actually not uncommon to see SYCL functor passed with many unused fields (Eigen is good example).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114080