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 <[email protected]> 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 <[email protected]> > wrote: >> >> 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 > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
