KDevelop LLDB support - GDB plugin refactor
ClosedPublic

Authored by qi437103 on May 26 2016, 4:21 PM.

Details

Summary

Extract common code from the GDB debugger plugin, which potentially can be reused by LLDB debugger plugin.
This refactor only touches the low-level plugin part. The UI related and other things (namely launch configuration and configpages) will be done in a separate RR later.

Changes done:

  • Rename namespace GDBMI to KDevDebugger::MI
  • Move MI parser code to common folder
  • Rename namespace GDBDebugger to KDevDebugger::GDB
  • Move code from GDBDebugger::DebugSession to KDevDebugger::DebugSessionBase and make KDevDebugger::GDB::DebugSession inherits from it
  • Move code from GDBDebugger::GDB to KDevDebugger::DebuggerBase and make KDevDebugger::GDB::GDB inherits from it
  • Move code from GDBDebugger::GdbFrameStackModel to KDevDebugger::MIFrameStackModel and make KDevDebugger::GDB::GdbFrameStackModel inherits from it
  • Move code from GDBDebugger::BreakpointController to KDevDebugger::BreakpointControllerBase and make KDevDebugger::GDB::BreakpointController inherits from it
  • Move code from GDBDebugger::VariableController to KDevDebugger::VariableControllerBase and make KDevDebugger::GDB::VariableController inherits from it
  • Move code from GDBDebugger::GdbVariable to KDevDebugger::MIVariable and make KDevDebugger::GDB::GdbVariable inherits from it
  • Move stty and stringhelper to common code

It's still unknown how much LLDB-MI comforms to the MI specification, so I might be moving too much code to common code, but I try to reuse as many code as possible when implementing LLDB support, and will move some GDB-specific code back when it's suitable.

Note: unit test requires protected access to several class in KDevPlatform, due to namespace changes, the friend class declaration in KDevPlatform are invalid now. You will need to change that in KDevPlatform:debugger/variable/variablecollection.h to get the unit test compile. Anyway, I don't it's a good idea to have friend class declaration in KDevPlatform, and I'm looking into fix that later.

Test Plan

Use existing unit tests, which should all pass, except 4-5 tests that are failing before these change.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
qi437103 updated this revision to Diff 4014.May 26 2016, 4:21 PM
qi437103 retitled this revision from to KDevelop LLDB support - GDB plugin refactor.
qi437103 updated this object.
qi437103 edited the test plan for this revision. (Show Details)
qi437103 added a reviewer: apol.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptMay 26 2016, 4:21 PM

Sorry for a really long diff :P
Refactor always touches many files. I'll try to avoid such lengthy thing in following up RRs.

apol edited edge metadata.May 27 2016, 3:21 PM

I think it's looking quite good, I'd say there's some naming ambiguities that we could solve now that could be best. This is about sharing MI-related code, this should be reflected in the names:

  • DebugSessionBase -> MIDebugSession
  • DebuggerBase -> MIDebugger
  • KDevDebugger -> MIDebugger

I'd think about it this way: a KDevDebugger would be something that every debugger would implement to be in KDevelop (for example a Ruby debugger or a Visual Studio debugger), but in this case neither of them could leverage it, because KDevDebugger is about implementing debuggers through MI-based protocols.

Yes, that makes sense. I wasn't aware of other debuggers when I first named them.

BTW, could you point me to the repo that holds those out-of-tree debugger implementations (like ruby or php or python)? I'd like to have a look at their code.

I rethought about the KDevDebugger namespace. It's maybe better to not change, as I'd consider it as an umbrella namespace for all debuggers in KDevelop. The MI prefix in class names is sufficient to say they are only for MI protocol based debuggers.

Also, it's kind of confusing to have the namespace using the same name as a class inside it.

So I suggest the following renaming in addition

  • KDevDebugger::GDB::GDB -> KDevDebugger::GDB::GdbDebugger
  • KDevDebugger::BreakpointControllerBase -> KDevDebugger::MIBreakpointController
  • KDevDebugger::VariableControllerBase -> KDevDebugger::MIVariableController
qi437103 updated this revision to Diff 4025.May 27 2016, 9:07 PM
qi437103 edited edge metadata.

Rename classes to better reflect that they are MI based debugger implementation

  • Rename DebuggerBase to MIDebugger
  • Rename DebugSessionBase to MIDebugSession
  • Rename VariableControllerBase to MIVariableController
  • Rename BreakpointControllerBase to MIBreakpointController
  • Rename KDevDebugger::GDB::GDB to KDevDebugger::GDB::GdbDebugger
apol added a comment.May 28 2016, 1:26 AM

I rethought about the KDevDebugger namespace. It's maybe better to not change, as I'd consider it as an umbrella namespace for all debuggers in KDevelop. The MI prefix in class names is sufficient to say they are only for MI protocol based debuggers.

Also, it's kind of confusing to have the namespace using the same name as a class inside it.

So I suggest the following renaming in addition

  • KDevDebugger::GDB::GDB -> KDevDebugger::GDB::GdbDebugger
  • KDevDebugger::BreakpointControllerBase -> KDevDebugger::MIBreakpointController
  • KDevDebugger::VariableControllerBase -> KDevDebugger::MIVariableController

It doesn't make sense to share a namespace between modules that will not share code. In fact, this namespace is probably unnecessary altogether, since it's just used to outline the implementation detail.

As for the other plugins, you can find them here: https://quickgit.kde.org/, look at those starting with kdev-* i.e. crossfire, xdebug. I don't know if there's more. scummos would know if there's something for python within kdev-python.

HTH

IMO at least a namespace to group all MI based debugger implementation is necessary.

I checked some other debugger implementations. The namespaces they use

  • kdev-crossfire: Crossfire
  • kdev-xdebug: XDebug
  • kdev-python: Python

What about KDevDebugger -> CppMIDebugger? I'm just trying to avoid using MIDebugger as both a namespace name and a class name.

apol added a comment.May 29 2016, 10:13 PM

What about KDevDebugger -> CppMIDebugger? I'm just trying to avoid using MIDebugger as both a namespace name and a class name.

I would call the namespace KDevMI. Note it's not cpp, as gdb works for many languages and so does lldb.

Yes, that sounds better!

qi437103 updated this revision to Diff 4043.May 30 2016, 4:40 AM

Rename namespace KDevDebugger to KDevMI

apol added a comment.May 30 2016, 11:35 AM

Ok, looks good. So what's the status now? You need to fix the friends in kdevplatform? Anything else missing?

The VariableCollection.h in kdevplatform needs fix for the unit test to compile. This would be another RR?

After that I think I can work on implementing LLDB classes using the split out common code.

apol added a comment.May 31 2016, 10:35 AM

The VariableCollection.h in kdevplatform needs fix for the unit test to compile. This would be another RR?

Yes, you need a separate one because it's in a different repository.
You can submit it now and we'll approve both patches.

This revision was automatically updated to reflect the committed changes.
rjvbb added a subscriber: rjvbb.Jun 21 2016, 8:52 AM

Very nice to see that things are moving a bit w.r.t. support for other debuggers. Lldb support would be a must for standalone use on OS X, of course.

It's still unknown how much LLDB-MI comforms to the MI specification

Yeah, that's a problem. IIRC I grappled with this a year or two ago and got some very basic lldb interaction going. But it's really not the ideal approach, which was confirmed on the lldb ML. For true lldb integration you'll want to become a front-end to liblldb yourself rather than going through the lldb driver. That's probably more work in the sense that it's a completely different approach but it may turn out to be faster to do this properly from scratch.