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

[Larisse Voufo]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130603/f3ea473b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: n3323-contextual-conversion-rev1.diff
Type: application/octet-stream
Size: 22334 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130603/f3ea473b/attachment.obj>