[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:364
       for (auto &I : Atomic64BitTypes)
         setOpenCLExtensionForType(I,
             "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics");
----------------
Anastasia wrote:
> I think this should be lifted into the above if statement? Although since we are adding the types conditionally we could even drop this code completely and let the developers just use the types as they are already added and made available by the frontend.
>  I think this should be lifted into the above if statement?

No, this is needed to be here as `atomic_double` added above.

>  Although since we are adding the types conditionally we could even drop this code completely and let the developers just use the types as they are already added and made available by the frontend.

I'm now worried about diagnostics  with that change:

```
tmp.cl:2:17: error: expected ';' after expression
    atomic_ulong a;
                ^
                ;
tmp.cl:2:5: error: use of undeclared identifier 'atomic_ulong'
    atomic_ulong a;
    ^
tmp.cl:2:18: error: use of undeclared identifier 'a'
    atomic_ulong a;
                 ^
tmp.cl:3:5: error: unknown type name 'atomic_uintptr_t'; did you mean 'atomic_uint'?
    atomic_uintptr_t u;
    ^~~~~~~~~~~~~~~~
    atomic_uint
note: 'atomic_uint' declared here
4 errors generated.
```
Is this expected behaviour when `cl_khr_int64_base_atomics` and 'cl_khr_int64_extended_atomics' not supported? I think we need o be more careful with reserved identifiers. However,  I didn't find explicit wording about it, but I'm pretty sure that all of these atomic types identifiers should be reserved. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058