[PATCH] Replacing one diagnostic with another

[Richard Smith]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130729/9e0d2690/attachment.html>