Re: [PATCH] Make dwarf_expr_context::stack an std::vector

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-09-14 18:43, Pedro Alves wrote:
> On 09/14/2017 05:31 PM, Simon Marchi wrote:
> 
>>> A couple nits and this is fine with me.
>>> 
>>>>    /* True if the piece is in memory and is known to be on the
>>>> program's stack.
>>>> @@ -114,7 +118,7 @@ struct dwarf_stack_value
>>>>  struct dwarf_expr_context
>>>>  {
>>>>    dwarf_expr_context ();
>>>> -  virtual ~dwarf_expr_context ();
>>>> +  virtual ~dwarf_expr_context () = default;
>>> 
>>> Couldn't we just remove the dtor?
>> 
>> I thought that this was necessary because there are subclasses that
>> might have destructors.  But now that I think about it more, I suppose
>> it would only be relevant if we destroyed instances of subclasses
>> through pointers to dwarf_expr_context?  All the instances of these
>> classes are static.
> 
> I actually didn't realize _this_ was the base class.  :-P
> But yeah, if we never want to delete via base pointer, the dtor
> could be non-virtual, and in which case it should be made
> protected, to ensure that really no code outside subclasses tries
> to call it directly.  (And in that case, you'd still use =default;)
> Really not a big deal, I'm fine with what you had.

Ok, well I've pushed and left the virtual dtor there just in case it's 
ever needed.  I don't think it changes anything at runtime.

Thanks,

Simon