[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Some relatively minor comments, overall direction seems good to me.



================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:67
 public:
   struct OpenCLOptionInfo {
     // Option starts to be available in this OpenCL version
----------------
In OpenCLExtensions.def, the order is: pragma, avail, core, opt.
In this class it is: avail, core, opt, pragma (for both the attributes and the constructor parameters).

There are very few usages of this, so the impact of this inconsistency is minor. However, because the conversion between unsigned and bool is implicit an error might go unnoticed for a long time.


================
Comment at: clang/lib/Basic/OpenCLOptions.cpp:89-91
+  OptMap.insert(                                                               \
+      std::make_pair(llvm::StringRef{#Ext},                                    \
+                     OpenCLOptionInfo{AvailVer, CoreVer, OptVer, Pragma}));
----------------
This could be done using `insert_or_assign(key, value)`.


================
Comment at: clang/lib/Basic/Targets.cpp:738
   };
-#define OPENCL_GENERIC_EXTENSION(Ext, Avail, Core, Opt)                        \
+#define OPENCL_GENERIC_EXTENSION(Ext, WithPragma, Avail, Core, Opt)            \
   defineOpenCLExtMacro(#Ext, Avail, Core, Opt);
----------------
For consistency with OpenCLExtensions.def.


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

https://reviews.llvm.org/D97052