[PATCH] Bugs in SemaOverload.cpp

[Alp Toker]

Hi Dmitry,

I agree the current setup isn't ideal.

An alternative to your approach is to convert this into an X macro table 
(or just a big rectangular array perhaps). Something like:

ImplicitConversions.def:

   IC(Kind, Name, Category, Rank):
   IC(Identity, "No conversion", Identity, Exact_Match)
   IC(Lvalue_To_Rvalue, "Lvalue-to-rvalue", Lvalue_Transformation, 
Exact_Match)
   ...


(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.)

What do you think?

Alp.


On 01/07/2014 14:56, Dmitry Babokin wrote:
> Hi rsmith,
>
> I've noticed that GetConversionCategory(), GetConversionRank() and GetImplicitConversionName() from lib/Sema/SemaOverload.cpp have a bunch of bugs, namely:
>   - GetConversionCategory() is missing cases for ICK_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);
>   - GetConversionRank() is missing the case for ICK_Zero_Event_Conversion;
>   - GetImplicitConversionName() doesn't have a comma after "Transparent Union Conversion";
>
> 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.
>
> Except style changes and typo fix (missing comma), the changes add case for missing records, please verify that they've got correct values.
>
> I have no commit rights, so I would appreciate if reviewer commits the patch. Thanks!
>
> http://reviews.llvm.org/D4354
>
> Files:
>    lib/Sema/SemaOverload.cpp
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-- 
http://www.nuanti.com
the browser experts