support labels in BreakpointViewer

[pvmm]

[m9710797]

Thanks, seems like a useful addition.

I didn't have time yet to try your patch, but already a quick question: Does the label replace the numeric value, or is it additional information? I assume that as a debugger-user, most often the label is most useful. But occasionally you still want to know the numeric value.

I'll start a review soon, but I'll likely won't have time to finish it today.

[pvmm]

If the user writes a numeric address in the address field, it will display it as a numeric address. Only if the user writes it as a symbolic address in the address field it will be displayed as a symbolic address. So it is entirely up to the user.

And internally the label is converted into a numeric address.

[m9710797]

Hi,
I did a quick review-pass, just looking for general c++ stuff. I did not yet try to understand what the code is actually doing. Nor did I try to run the code. And I won't have time for that today.

[m9710797]

If the user writes a numeric address in the address field, it will display it as a numeric address. Only if the user writes it as a symbolic address in the address field it will be displayed as a symbolic address. So it is entirely up to the user.

OK. What if the breakpoint wasn't (manually) set via the debugger, but via some other way, e.g. via the openMSX console, or via some Tcl script? I think it would be nice if also for such breakpoints we try to show them as labels (but then it's more important to still be able to see the numeric value).

I actually don't know how the disassembly view works. Does e.g. the disassembly of JP 0x1234 get resolved to a label if a label for 0x1234 is available?

I see in the Symbol class that symbols can have a type. Maybe this substitution should only be done for symbols of type JUMPLABEL?

[pvmm]

OK. What if the breakpoint wasn't (manually) set via the debugger, but via some other way, e.g. via the openMSX console, or via some Tcl script? I think it would be nice if also for such breakpoints we try to show them as labels (but then it's more important to still be able to see the numeric value).

I could put the address visible in another column, but it will clutter the interface a little and may not be useful in most cases.

I actually don't know how the disassembly view works. Does e.g. the disassembly of JP 0x1234 get resolved to a label if a label for 0x1234 is available?

It is.

I see in the Symbol class that symbols can have a type. Maybe this substitution should only be done for symbols of type JUMPLABEL?

Types exist, but are not used anywhere AFAIK. You cannot set a symbol that is not a JUMPLABEL in the Symbol Manager right now.

[pvmm]

I see in the Symbol class that symbols can have a type. Maybe this substitution should only be done for symbols of type JUMPLABEL?

And this also: there is no function in SymbolTable that helps me do this kind of type filtering. And then some patches that should be simple start getting too big and out of scope I am afraid.

[m9710797]

I agree that you only occasionally want to see the numeric value. So cluttering the output with an extra column is not a good solution. Maybe a popup message could be a solution (when you hover the mouse pointer over the label)? I don't know if that is easy to do in a table in Qt.

IMHO it would be nice if addresses/labels behave the same everywhere (as much as possible) in the debugger. So in the disassembly view, in the breakpoint viewer, any other widgets?

(If you want I can already accept your patch before these changes (popup message). But after you fixed yesterdays review comments.)

[pvmm]

IMHO it would be nice if addresses/labels behave the same everywhere (as much as possible) in the debugger. So in the disassembly view, in the breakpoint viewer, any other widgets?

Maybe in Main Memory when VARIABLELABEL is properly implemented as a popup message.

[pvmm]

BreakpointViewer::setTextField() now has a tooltip parameter.

[m9710797]

Thanks!

[pvmm]

Hosting screenshot for MSX Resource Center. 😉