Re: [PATCH v2] Replace the symbol needs evaluator with a parser

[Tom Tromey <tom@tromey.com>]

>>>>> "Zoran" == Zoran Zaric <Zoran.Zaric@amd.com> writes:

Zoran> This patch addresses a design problem with the symbol_needs_eval_context
Zoran> class. It exposes the problem by introducing two new testsuite test
Zoran> cases.

Thank you for the patch.

Zoran> This is clearly a wrong result and it causes the debugger to crash.

One question I have here is whether we even need this symbol-needs-frame
stuff.  What if, instead, we just had gdb throw an exception in the
situation where a frame is needed but not available?  Then we could get
rid of the asserts and gdb would simply print an error rather than
crash.

Can you look to see if that is feasible?  The main advantage I see here
is that this approach would avoid the problem we sometimes have of
updating one DWARF expression-decoder and then forgetting to update the
others...

Zoran> A more desired long term design would be to have one class that deals
Zoran> with parsing of the DWARF expression, while there would be a virtual
Zoran> methods that deal with specifics of some DWARF operations. Then that
Zoran> class would be used as a base for all DWARF expression parsing mentioned
Zoran> at the beginning.

What I did in gimli is have a parser that converts the DWARF expression
to an internal form.  Then users of the API can decide how to manipulate
this form -- dump it, evaluate it, etc.  This separates the low-level
parsing bits from the decisions about what operations to perform.

Zoran> gdb/ChangeLog:

Zoran>         * dwarf2/loc.c (class symbol_needs_eval_context): Remove.

This is PR gdb/10592 and so that should be mentioned in both ChangeLogs.

thanks,
Tom