On 15 May 2015 at 10:39, Dodji Seketeli <do...@redhat.com> wrote:
> Manuel López-Ibáñez <lopeziba...@gmail.com> writes:
>> -/* Expand the location of this diagnostic. Use this function for
>> consistency. */
>> +/* Return the location associated to this diagnostic. WHICH specifies
>
> Here, I think only the 'W' (in WHICH) should be uppercase.
I'm following the convention that parameter names are uppercase in the
description of functions. I can also say "Parameter WHICH specifies"
to be more explicit.
>> /* The type of a text to be formatted according a format specification
>> along with a list of things. */
>> struct text_info
>> {
>> +public:
>
> As this is a struct, the 'public' here is not necessary, as the members
> are public by default.
I have a very poor memory for such details ;), since we are using
'private:' already, does it really hurt to be explicit and use
'public:' here?
>> const char *format_spec;
>> va_list *args_ptr;
>> int err_no; /* for %m */
>> - location_t *locus;
>> void **x_data;
>> +
>> + inline location_t & set_location (unsigned int index_of_location)
>
> I think it's less surprising to have the this function take two
> parameters: The index_of_location and the new new location.
OK.
> OK to commit with the changes above.
Thanks. If you insist on the two first changes, I'll do them, I just
want to confirm those.
Cheers,
Manuel.
