[PATCH] D120262: [OpenCL] Handle TypeExtensions in OpenCLBuiltinFileEmitter

[Pedro Olsen Ferreira via Phabricator via cfe-commits]

arkangath added inline comments.


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1183
+      SmallVector<StringRef, 2> ExtVec;
+      TypeExt.split(ExtVec, " ");
+      for (const auto Ext : ExtVec) {
----------------
Just in case if relevant, your "KeepEmpty" will default to true here.
I don't know if it is possible or not (not enough context for me), but could the .td file have "Extension0  Extention1" (two spaces) that could lead to one empty StringRef here?


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1191
+  // Emit the #if when at least one extension is required.
+  if (!ExtSet.empty()) {
+    OS << "#if ";
----------------
Arguably it _may_ be better to early-return here instead of increasing the indentation, but I'm not asking for a change; just suggesting.
The assigned values into OptionalEndif could be just returned directly rather than storing in a local variable, unless you're predicting further changes to this function benefit from this temporary existing.


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

https://reviews.llvm.org/D120262