[PATCH] D96860: [OpenCL] Add declarations with enum/typedef args

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks!



================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:933
+  // The OpenCL 3.0 specification defines these with a "gentype" argument indicating any builtin
+  // type or user-defined type, which cannot be represented currently.  Hence we slightly diverge
+  // by providing only the following overloads with a void pointer.
----------------
I would suggest changing this slightly to:

`which cannot be represented currently` -> `which cannot be represented in C based languages`

otherwise, it feels like there would be a way to represent it later. I just want to make sure it is clear that this is not a limitation of Tablegen or anything else in the implementation but rather conceptual issue.


================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:1103
   }
   foreach TypePair = [[AtomicInt, Int, Int], [AtomicUInt, UInt, UInt],
                       [AtomicLong, Long, Long], [AtomicULong, ULong, ULong],
----------------
svenvh wrote:
> Anastasia wrote:
> > I think this should be renamed now, but we can do as a separate change.
> > 
> > I would suggest using something like `OverloadTypes` instead...
> Agreed that renaming belongs to a separate patch, we can probably bikeshed over the actual name. `TypeTuple` would already be better than `TypePair` for this case.  I find `OverloadTypes` a bit ambiguous (I guess with "Overload" you intend to refer to a particular instance of the overloaded function, but not sure if it's common to use "overload" as a noun).  `ReturnTypeAndArgumentTypes` or `OverloadedFunctionTypes` seem more accurate but a bit unwieldy...
Yes, we can use more domain-specific names I would prefer those to `TypeTuple`.


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

https://reviews.llvm.org/D96860