[PATCH] D94188: [OpenCL] Documentation for experimental C++ libraries support

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Looks good overall.



================
Comment at: clang/docs/OpenCLSupport.rst:104
+Clang provides the following new WIP features for the developers to experiment
+with and provide early feedback or contribute with further improvements.
+Feel free to contact us on `cfe-dev
----------------
Remove "with".


================
Comment at: clang/docs/OpenCLSupport.rst:115
+
+It is currently possible to use `type_traits` from C++17 in the kernel sources
+if the following clang extensions are enabled ``__cl_clang_function_pointers``
----------------
s/use/include/


================
Comment at: clang/docs/OpenCLSupport.rst:116
+It is currently possible to use `type_traits` from C++17 in the kernel sources
+if the following clang extensions are enabled ``__cl_clang_function_pointers``
+and ``__cl_clang_variadic_functions``, see :doc:`LanguageExtensions` for more
----------------
s/if/when/


================
Comment at: clang/docs/OpenCLSupport.rst:134
+  #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
+  #include "type_traits"
+  #pragma OPENCL EXTENSION __cl_clang_function_pointers : disable
----------------
I'd use < and > instead of ". It should work too, right? It would be closer to standard C++ usage.


================
Comment at: clang/docs/OpenCLSupport.rst:138
+ 
+  typedef std::make_signed<unsigned int>::type sint_t;
+        
----------------
Two things:
- identifier ending with `_t` are reserved for the compiler I believe.
- to show usage of modern C++, this could be rewritten into `using sint = std::...`.


================
Comment at: clang/docs/OpenCLSupport.rst:151
+Note that `type_traits` is a header only library and therefore no extra
+linking step for the standard libraries is required.
----------------
s/for/against/


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

https://reviews.llvm.org/D94188