[PATCH] Bugs in SemaOverload.cpp

[Alp Toker]

On 01/07/2014 22:00, Alp Toker wrote:
> 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)
>   ...
>

In other words, that'd be similar to what we already do for overloaded 
operators:

   include/clang/Basic/OperatorKinds.def

Alp.

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