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

[Pedro Alves <palves at redhat dot com>]

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.

> 
>>>
>>>    void push_address (CORE_ADDR value, bool in_stack_memory);
>>>    void eval (const gdb_byte *addr, size_t len);
>>> @@ -123,11 +127,7 @@ struct dwarf_expr_context
>>>    bool fetch_in_stack_memory (int n);
>>>
>>>    /* The stack of values, allocated with xmalloc.  */
>>
>> "xmalloc" reference is stale.
> 
> Ok.
> 

Thanks,
Pedro Alves