[PATCH] Replacing one diagnostic with another

Aaron Ballman aaron at aaronballman.com
Mon Jul 29 18:33:22 PDT 2013


Thanks!  I've done so in r187400; I also changed the enumerants to
have a prefix since they are no longer limited to a single cpp file.

~Aaron

On Mon, Jul 29, 2013 at 1:49 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Please move the diagnostic kind enum into a header and use it from both
> SemaType and SemaDeclAttr. Otherwise, looks fine.
>
>
> On Mon, Jul 29, 2013 at 6:20 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Ping
>>
>> On Wed, Jul 24, 2013 at 9:49 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>> > 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
>> >>>>
>> >>>>
>> >>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>



More information about the cfe-commits mailing list