[PATCH] D51341: [HEADER] Overloadable function candidates for half/double types

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: lib/Sema/SemaOverload.cpp:6063
+  // OpenCL: A candidate function that uses extentions that are not enabled or
+  // supported is not viable.
+  if (getLangOpts().OpenCL) {
----------------
sidorovd wrote:
> Anastasia wrote:
> > I would imagine if extension isn't supported the candidate function with data type defined by extension shouldn't be available at all during compilation?
> > 
> > Also is there any good way to generalize this for all types and extensions including vendor ones that are added with the pragmas?
> > https://clang.llvm.org/docs/UsersManual.html#opencl-extensions
> > I would imagine if extension isn't supported the candidate function with data type defined by extension shouldn't be available at all during compilation?
> 
> Yes, you are right.
> 
> > Also is there any good way to generalize this for all types and extensions including vendor ones that are added with the pragmas?
> https://clang.llvm.org/docs/UsersManual.html#opencl-extensions
> 
> There might be a way but I can't properly answer this question right now. By default types and extensions aren't associated to each other.
We have a map of types and associated extensions with them in `OpenCLTypeExtMap` in Sema.h... not sure what would be the cost of such generic check though.


================
Comment at: test/SemaOpenCL/half-double-overload.cl:18
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note at -1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);
----------------
sidorovd wrote:
> Anastasia wrote:
> > Wondering if better message could be:
> >   candidate unavailable as it requires OpenCL extension to be disabled
> > 
> > Could it also print the extension name?
> Sounds good. And I'd prefer to split it into another patch, because it would affect unrelated to the change tests and also require changes in Sema to allow extension name printing. 
Ok, makes sense for this to be a separate change!


https://reviews.llvm.org/D51341