[PATCH] N3323 -- Tweak to certain C++ contextual conversions

[Richard Smith]

Patch LGTM, thanks!

On Mon, Jun 3, 2013 at 3:35 PM, Larisse Voufo <lvoufo at google.com> wrote:
>
> I am attaching a revision of the patch based on the last comments.
>
>
> On Thu, May 30, 2013 at 5:54 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>>
>> On Thu, May 30, 2013 at 4:35 PM, Larisse Voufo <lvoufo at google.com> wrote:
>>>
>>> Please see attached.
>>
>>
>> This looks really good.
>>
>> Something funny has happened around < >s for templates. I've run the patch
>> through clang-format for you (attached).
>
>
> Thanks. It looks like my editor cannot tell template arguments' brackets
> from binary operators, and defaults into the later option. I'll be more
> careful about that.
>
>>
>>
>> +                // Fixme: this assumes that both types will be equally
>> +                // restrict-qualified.
>>
>> s/Fixme/FIXME/
>>
>>
>>
>> +                if (ToType.isMoreQualifiedThan(CurToType))
>> +                  ToType = CurToType;
>>
>> The standard doesn't seem very clear on this, but I think you should merge
>> qualifiers here rather than taking the more-qualified type. If we have a
>> conversion to 'const T' and a conversion to 'volatile T', our current choice
>> between them seems to depend on the order in which we see them.
>>
> Ah. Good point. Following this train or reasoning, we should be looking for
> the least common qualifier between the conversion candidates.
> However, as you pointed out earlier, the standard does suggest to just drop
> the qualifiers. I have made the change (cf. attached).
>
>>
>>
>> +      if (ConvTemplate)
>> +        AddTemplateConversionCandidate(ConvTemplate, FoundDecl,
>> ActingContext,
>> +                                       From, ToType, CandidateSet);
>>
>> It looks like ViableConversions won't contain any conversion operator
>> templates at this point. However, we should probably be performing overload
>> resolution on the complete set of conversions, not just on the ones we
>> matched earlier,
>
>
> Right. I have changed the meaning of ViableConversions for C++1y so that it
> contains *potentially* viable conversions, i.e. including conversion
> templates as well (again, cf. attached).
>
>>
>> for oddball cases like:
>>
>>   struct A {
>>     operator int() &&;
>>     template<typename T> operator T();
>>   };
>>   void f(A a) {
>>     switch (a) {} // should presumably call templated conversion operator
>> to convert to int.
>>   }
>>   struct B {
>>     operator bool() &&;
>>     operator void*();
>>   };
>>   void g(B b) {
>>     switch (b) {} // should presumably call 'b.operator void*()' then
>> convert result to bool
>>   }
>>
>> The patch could do with more testcases, covering more exotic situations
>> such as the above, and the new error cases.
>
> Sure. Working on that. I've added a unit tests. More test cases are welcome.
> :)
> The above example is now covered.
>
>>
>>
>> Thanks!
>> Richard
>
>
>