[PATCH] Replacing one diagnostic with another

[Aaron Ballman]

I agree -- the parameter 1 designation was not ideal.  New patch adds
the additional diagnostic sans parameter position.  Assuming this is
acceptable, I'll remove the duplicate integer diagnostic and replace
it with the new diagnostic.

Thanks!

~Aaron

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