[PATCH] D56735: [OpenCL] Fix overloading ranking rules to work correctly for address space conversions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D56735#1359590 <https://reviews.llvm.org/D56735#1359590>, @Anastasia wrote:

> In D56735#1358222 <https://reviews.llvm.org/D56735#1358222>, @rjmccall wrote:
>
> > Is there a reason not to just do `T1.getQualifiers() == T2.getQualifiers()`?
>
>
> I tried this but it causes ObjC test to fail test/SemaObjCXX/arc-overloading.mm.
>
>   38 // Simple overloading
>   39 int &f2(id __strong const *); // expected-note{{candidate function}}
>   40 float &f2(id __autoreleasing const *); // expected-note{{candidate function}}
>   41 
>   42 void test_f2() {
>   43   id __strong *sip;
>   44   id __strong const *csip;
>   45   id __weak *wip;
>   46   id __autoreleasing *aip;
>   47   id __unsafe_unretained *uip;
>   48 
>   49   // Prefer non-ownership conversions to ownership conversions.
>   50   int &ir1 = f2(sip);
>   51   int &ir2 = f2(csip);
>   52   float &fr1 = f2(aip);
>   53 
>   54   f2(uip); // expected-error{{call to 'f2' is ambiguous}}
>   55 }
>   
>
> Clang fails to resolve the overloads:
>
>   error: 'error' diagnostics seen but not expected: 
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 50: call to 'f2' is ambiguous
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 52: call to 'f2' is ambiguous
>   error: 'note' diagnostics seen but not expected: 
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 39: candidate function
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 40: candidate function
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 39: candidate function
>     File /data/llvm/llvm/tools/clang/test/SemaObjCXX/arc-overloading.mm Line 40: candidate function
>   
>
> Since CVR quals for both overloads of `f2` are the same it didn't continue ranking the other qualifiers before. But now when it continues looking at the other qualifiers it finds that `__strong` and `__autoreleasing` are disjoint and exits at the following return statement in `CompareQualificationConversions`:
>
>   } else {
>     // Qualifiers are disjoint.
>     return ImplicitConversionSequence::Indistinguishable;
>   }
>   
>
> I feel the logic here doesn't apply to this case because it doesn't take into account that for the passed in function argument there is no ownership qualification conversion for one of the overloads. But I don't have a good idea how to fix this easily other than just keep fast pathing  ObjC qual here.
>
> May be if we add a separate qualification conversion for ObjC ownership qualification only we will end up with more conversion steps for one overload and therefore it will be possible to resolve the ambiguity? Do you have any other suggestions or do you think someone else could help?


For now, let's just remove the ownership qualifiers before comparing and leave a comment explaining that considering them here seems to interfere with the ARC overload rule.


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

https://reviews.llvm.org/D56735