Please move the diagnostic kind enum into a header and use it from both SemaType and SemaDeclAttr. Otherwise, looks fine.<br><br><div class="gmail_quote">On Mon, Jul 29, 2013 at 6:20 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ping<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Jul 24, 2013 at 9:49 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> I agree -- the parameter 1 designation was not ideal.  New patch adds<br>
> the additional diagnostic sans parameter position.  Assuming this is<br>
> acceptable, I'll remove the duplicate integer diagnostic and replace<br>
> it with the new diagnostic.<br>
><br>
> Thanks!<br>
><br>
> ~Aaron<br>
><br>
> On Tue, Jul 23, 2013 at 8:50 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:<br>
>> Ah, hm, yeah. Passing in the number of total arguments is rather annoying. I guess duplicate diagnostics is the best way to go, since in almost every case the call site will know which one to use. I still find it better for the user than saying "parameter 1" for a one-parameter attribute.<br>

>><br>
>> Jordan<br>
>><br>
>><br>
>> On Jul 23, 2013, at 14:11 , Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>><br>
>>> From what I can tell, that makes using the diagnostic rather complex.<br>
>>> Perhaps it could be split into two diagnostics using the same list?<br>
>>><br>
>>> def err_attribute_argument_n_type : Error<<br>
>>>  "%0 attribute requires parameter %1 to be %select{int or bool|an integer "<br>
>>>  "constant|a string|an identifier}2">;<br>
>>><br>
>>> def err_attribute_argument_type : Error<<br>
>>>  "%0 attribute requires %select{int or bool|an integer "<br>
>>>  "constant|a string|an identifier}1">;<br>
>>><br>
>>> I'm not keen on the duplication, but I can't seem to find a way to<br>
>>> make plural work that doesn't require the caller to pass in the<br>
>>> expected number of arguments along with the argument the diagnostic is<br>
>>> for.<br>
>>><br>
>>> ~Aaron<br>
>>><br>
>>> On Tue, Jul 23, 2013 at 12:02 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:<br>
>>>> Mm...seems a bit weird to index the attribute parameters when there's only one. Can we special case that with a %plural?<br>
>>>><br>
>>>> Jordan<br>
>>>><br>
>>>> On Jul 23, 2013, at 8:47 , Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>>>><br>
>>>>> The err_attribute_not_string error seems like it can be subsumed by<br>
>>>>> the err_attribute_argument_n_type diagnostic; the main difference<br>
>>>>> between the two being that the latter specifies an argument position<br>
>>>>> while the former does not.<br>
>>>>><br>
>>>>> This patch performs the replacement, but I wanted to make sure it was<br>
>>>>> acceptable.  If the consensus is that this is fine, I will commit a<br>
>>>>> separate patch that does the same for err_attribute_argument_not_int.<br>
>>>>><br>
>>>>> ~Aaron<br>
>>>><br>
>>>>> _______________________________________________<br>
>>>>> cfe-commits mailing list<br>
>>>>> <a href="mailto:cfe-commits@cs.uiuc.edu">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/mailman/listinfo/cfe-commits</a><br>
>>>><br>
>>>><br>
>><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">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/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br>