On Dec 10, 2013, at 9:20 AM, Jordan Rose <[email protected]> wrote:

> Thanks, this is better. Still have some comments though.
> 
> On Dec 9, 2013, at 14:04 , Fariborz Jahanian <[email protected]> wrote:
> 
>> + def err_objc_bridged_related_unknown_method : Error<
>> + "you can't convert %0 to %1, without using an existing "
>> + "%select{class|instance}2 method for this conversion">;
> 
> I still don't see what this diagnostic adds. If the framework doesn't have a 
> method name for one of the conversion directions, that probably means there's 
> no one good way to perform this conversion. There may be no way to perform 
> the conversion, although it's strange that they would be marked related, then.
> 
> Also, diagnostics generally don't have "you" except in the fix-it bit ("did 
> you mean...").
> 
> How about just "%0 cannot be directly converted to %1”?

I improved the text. I also want to mention that user may have forgotten to 
specify a selector name in the attribute.

> 
> 
>>       Diag(Loc, diag::err_objc_bridged_related_unknown_method)
>> -        << SrcType << DestType << RelatedClass->getName() << 0;
>> +        << SrcType << DestType << 0;
> 
> Here and elsewhere I think it makes sense to use false/true rather than 0/1 
> for the select parameter. If not that, then perhaps a local enum { IsClass = 
> 0, IsInstance };

Ok. 

In r196950.
- Fariborz

> 
> Jordan

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to