Ping On Wed, Jul 24, 2013 at 9:49 AM, Aaron Ballman <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 >>>>> [email protected] >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>> >>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
