<div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">I like Alp's suggestion of a .def file: the duplication and distribution of this information has been a (low level) annoyance for quite some time and it'd be good to properly fix it.</span><div class="" style="font-family:arial,sans-serif;font-size:13px">
</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 2, 2014 at 1:09 AM, Dmitry Babokin <span dir="ltr"><<a href="mailto:babokin@gmail.com" target="_blank">babokin@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Alp,<div><br></div><div>To me both approaches look good, as they are eliminating possibility of future errors. I don't have preference for one or another. I think it's up to file maintainers (Richard?) to decide what approach to follow.</div>

<div><br></div><div>By the way, my patch also contains fixes for missing cases in these routines. Are these fixes matches with what you have in your changes? Do they look correct to you?</div><span class="HOEnZb"><font color="#888888"><div>
<br></div><div>Dmitry.</div>
</font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 1, 2014 at 11:33 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
On 01/07/2014 22:00, Alp Toker wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Dmitry,<br>
<br>
I agree the current setup isn't ideal.<br>
<br>
An alternative to your approach is to convert this into an X macro table (or just a big rectangular array perhaps). Something like:<br>
<br>
ImplicitConversions.def:<br>
<br>
  IC(Kind, Name, Category, Rank):<br>
  IC(Identity, "No conversion", Identity, Exact_Match)<br>
  IC(Lvalue_To_Rvalue, "Lvalue-to-rvalue", Lvalue_Transformation, Exact_Match)<br>
  ...<br>
<br>
</blockquote>
<br></div>
In other words, that'd be similar to what we already do for overloaded operators:<br>
<br>
  include/clang/Basic/<u></u>OperatorKinds.def<span><font color="#888888"><br>
<br>
Alp.</font></span><div><div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
(We've made this change in our clang derivative, which implements several new conversion kinds, and the above format is serving us well because it also acts as documentation for the language extension, making it easy to see the ranks at a glance.)<br>


<br>
What do you think?<br>
<br>
Alp.<br>
<br>
<br>
On 01/07/2014 14:56, Dmitry Babokin wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi rsmith,<br>
<br>
I've noticed that GetConversionCategory(), GetConversionRank() and GetImplicitConversionName() from lib/Sema/SemaOverload.cpp have a bunch of bugs, namely:<br>
  - GetConversionCategory() is missing cases for ICK_<u></u>TransparentUnionConversion, ICK_Writeback_Conversion and ICK_Zero_Event_Conversion cases (ok to not handle ICK_Num_Conversion_Kinds, though would be good to report an error);<br>


  - GetConversionRank() is missing the case for ICK_Zero_Event_Conversion;<br>
  - GetImplicitConversionName() doesn't have a comma after "Transparent Union Conversion";<br>
<br>
I think it's better to rewrite these functions using switch, so adding new ICKs would trigger a warning or an error if it's not handled in these functions.<br>
<br>
Except style changes and typo fix (missing comma), the changes add case for missing records, please verify that they've got correct values.<br>
<br>
I have no commit rights, so I would appreciate if reviewer commits the patch. Thanks!<br>
<br>
<a href="http://reviews.llvm.org/D4354" target="_blank">http://reviews.llvm.org/D4354</a><br>
<br>
Files:<br>
   lib/Sema/SemaOverload.cpp<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br>
</blockquote>
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>