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
