[PATCH] D107648: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

Anastasia Stulova via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 6 09:41:17 PDT 2021


Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:248
 def warn_option_invalid_ocl_version : Warning<
-  "OpenCL version %0 does not support the option '%1'">, InGroup<Deprecated>;
+  "%select{OpenCL C|C++ for OpenCL}0 version %1 does not support the option '%2'">, InGroup<Deprecated>;
 
----------------
Since we have a number of diagnostics now that print the current lang version using this pattern do you think we could instead add a helper function that prints the full current OpenCL version spelling. We could add something similar to `LangOptions::getOpenCLVersionTuple()` so let's say `LangOptions::getOpenCLVersionString()` that could use a tuple-helper internally. Then we could change such diagnostics to something like:

`%0 does not support the option '%1'`

This could help us simplifying the source code and make sure the diagnostic wording is always consistent.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2956
 def err_attribute_requires_opencl_version : Error<
-  "%0 attribute requires OpenCL version %1%select{| or above}2">;
+  "%0 attribute requires %select{OpenCL C|C++ for OpenCL}1 version %2%select{| or above}3">;
 def err_invalid_branch_protection_spec : Error<
----------------
Let's simplify this to something like:


```
attribute %0 is supported in the OpenCL version %1%select{| onwards}3
```


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10151
 def ext_opencl_ext_vector_type_rgba_selector: ExtWarn<
-  "vector component name '%0' is an OpenCL C version 3.0 feature">,
+  "vector component name '%0' is %select{an OpenCL C|a C++ for OpenCL}1 version %2 feature">,
   InGroup<OpenCLUnsupportedRGBA>;
----------------
How about changing to:

`vector component name '%0' is a feature from OpenCL version 3.0 onwards`

or otherwise, we would need to enumerate all versions where it is supported which is not scalable.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7377
+    S.Diag(AL.getLoc(), diag::warn_opencl_attr_deprecated_ignored)
+        << AL << 0 << "2.0";
 }
----------------
I feel that this condition has assumed OpenCL 2.0 which was not correct. We need to render the current version correctly through the `LangOptions::getOpenCLVersionTuple()` helper.


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

https://reviews.llvm.org/D107648



More information about the cfe-commits mailing list