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

Zoran Zaric via Gdb-patches
2021-04-28T15:56:09Z

>>> 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
>

I looked even deeper in this issue and the problem is that both frame
and compilation unit information are valid to be nullptr in some cases,
but for certain operations they need to be present and if they are not
the evaluator needs to bail out.

There seems to be a couple of reason why they can be nullptr:

- some preexisting API will call the get_selected_frame (_("No frame
selected.")) if the frame is nullptr.

- evaluator can call another evaluator recursively that might also not
need the frame or compilation unit information or may bailout it they
are needed but not present.

- the composite and implicit pointer location description carry the
context information outside the evaluator which might never be used but
if they are the exception will be thrown.

There might be some other complications, but these seem to be the main ones.

Zoran