Re: [RFA 19/22] Convert tid_range_parser to class

[Pedro Alves <palves at redhat dot com>]

On 09/30/2016 03:07 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Whoops, we ended up duplicating work here.
> 
> Oops :) But no big deal.
> 
> Pedro> Additional differences compared to yours are:
> Pedro> - "bool" instead of "int".
> Pedro> - "m_" prefixes.
> 
> Aha, the m_ prefix.  Cancel that earlier question...
> 
> Pedro> Let me know what you think.  It's super fine with me to rebase
> Pedro> mine on top of yours.  Did you have a follow up patch for
> Pedro> get_number_or_range too, perhaps?
> 
> I don't.  How about I just drop my patch?  That seems simplest to me.

OK.  I'll look at your patch in more detail and see what could be
merged in.  I'll add your name to the ChangeLog.

> 
> Pedro>  void
> Pedro> -init_number_or_range (struct get_number_or_range_state *state,
> Pedro> -		      const char *string)
> Pedro> +number_or_range_parser::init (const char *string)
> Pedro>  {
> Pedro> -  memset (state, 0, sizeof (*state));
> Pedro> -  state->string = string;
> Pedro> +  memset (this, 0, sizeof (*this));
> 
> I think it's better to do explicit initialization.

I'll do this.

> This will bite if this class ever is changed to have a vtable (unlikely
> but it's better, IMO, to set a good example).

Yeah, we'd notice bad things immediately, but I agree with setting
a good example.

Thanks,
Pedro Alves