Re: [PATCH 03/43] Move frame context info to dwarf_expr_context

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

>> On an unrelated topic, things like ensure_have_frame always worry me
>> because I'm scared I'll forget to call it where necessary.  An
>> alternative could be to make sure we get the frame through a getter that
>> throws if frame == nullptr.
>>
>> Basically, a tiny class (could be internal to dwarf_expr_context):
>>
>> struct frame_safe
>> {
>>    frame_safe (frame_info *frame)
>>      : m_frame (frame)
>>    {}
>>
>>    frame_info *frame (const char *op)
>>    {
>>      if (m_frame == nullptr)
>>        ... throw ...
>>
>>      return m_frame;
>>    }
>>
>> private:
>>    frame_info *m_frame;
>> };
>>
>> dwarf_expr_context would have a `frame_safe m_frame` field and a
>> `frame (const char *op)` getter that just does
>> `return m_frame->frame (op)`.
>>
>> This way, it's impossible to get the frame and forgetting an
>> ensure_have_frame call.
> 
> True, but then you end up with even more "switch" statements in different parts of the gdb that one needs to update every time they add a new operation and this is one of the problems with the current design as is.

I don't understand what you mean, why it would mean adding more switch
statements.  In practice, it would just mean changing `this->frame` to
`this->frame ()`.

> This would force us to add two new places (one for frame and one for compilation unit) and there will be more in the future when we add a concept of a thread to the context.
> 
> At least with the current implementation the implementer of the case statement that process that operation needs to know which part of the context has to be present for that operation, which is clearly defined in the DWARF standard as part of that operations definition (or at least it will be after our extensions).
> 
> Another problem with the new class approach is that it makes the information about what each operation needs convoluted and hidden which in my mind adds even bigger chance of someone forgetting to add their operation to the query calls.

What do you mean by "add their operation to the query calls"?

I don't think see how it helps to make it explicit which operation needs
which part of the context.  If an operation needs a frame, it will call
`this->frame ()`  which will throw if the context does not have a
frame.

Simon