I have been giving study to the private and public classes for
break/watchpoints and I have some ideas for how to improve them. I am
looking for comments from the community on this now to avoid wasted work
and invite suggestion for improvements and any objections you may have.
Some problems with things as they stand:
1. The Watchpoint and SBWatchpoint classes share very little
implementation with the Breakpoint and SBBreakpoint classes. Both
Breakpoint and Watchpoint inherit from Stoppoint, but that class
provides very little functionality and not much of an interface. This is
both a waste and a potential hazard (in the sense that we run the risk
of allowing breakpoints and watchpoints to evolve in parallel thus
complicating the API and making things more difficult to merge later).
2. The SBWatchpoint class lacks feature parity with SBBreakpoint (e.g.
no callback implementation in SBWatchpoint).
3. The Breakpoint class has grown an ecosystem of support classes which
are unnecessarily specific to it. For example, I don't see a huge
reason to maintain a totally distinct implementation for BreakpointList
and WatchpointList.
4. All of these classes are "old school" (not necessarily bad, but
potentially a problem). For example:
a. lots of const char* running around.
b. DISALLOW_COPY_AND_ASSIGN(BreakpointEventData) to make ctors and
such private rather than using ctor() = delete (which provides better
compiler errors)
c. use of Error& args in function signatures as opposed to
Expected<ReturnType>.
d. callback implementation uses function pointers (an ever confusing
topic, especially for new programmers) where I think we could use
templated methods (or just a parent Callback class) to significant effect.
5. Confusing or simply wrong documentation (e.g.
Breakpoint::ResolveBreakpointInModules has arguments marked as
@param[in] when the description of those arguments clearly indicates
that they will be the output of the function).
6. We simply don't have a Finishpoint class (triggers callback at the
conclusion of the function or scope)
My plan:
I would like to refactor this in a few phases.
Phase 1: Low hanging fruit
--------------------------
* Kick as much functionality as is reasonable from Breakpoint up to
Stoppoint. At a minimum I would expand the Stoppoint API to include
features which should logically be available to Breakpoint, Watchpoint,
a hypothetical Finishpoint, and any future *point class. I would also
try to refactor the Breakpoint support classes (e.g. BreakpointID) to
derive from a new class (i.e. StoppointID) and kick functionality up the
chain there as well. This would include methods like GetDescription,
SetOptions, GetOptions, AddName, and so on.
* Review and clean the source docs
* Expand test coverage where that seems easy (perhaps there are tests
hiding somewhere I didn't see?)
I would try to break private APIs minimally if at all. SB api will not
be broken.
This should go a long ways toward resolving problems 1, 2, 3, and 5.
Phase 2: Restructure / Modernize the Private API / Implementation
-----------------------------------------------------------------
* Change Error& out parameters to Expected<ReturnType>.
* Get rid of all the const char* vars / args in favor of a better string
type (which?)
* Prefer explicitly deleted copy ctor / assignments over multiline macro
DISALLOW_COPY_AND_ASSIGN
* Try to reduce the (perhaps excessive) use of friendship between the
support classes. For instance, is it necessary to give
BreakpointLocation access to private data members from
BreakpointLocationList, Process, BreakpointSite, and
StopInfoBreakpoint? Wouldn't it be better to expand the functionality
of those other classes?
A more significant change could be a rewrite of the callback functionality.
There are some problems with the way callbacks are implemented in terms
of maintainability and extensibility. I think that using templates and
simple inheritance we could shift to using callback objects instead of
function pointers. I have a crude code sketch of this plan in the works
now, and I will submit that if there is interest in this idea.
Essentially, the idea is to let the user define their own Breakpoint or
Watchpoint (or whatever) child class which overrides a pure virtual run
method from a parent StoppointCallback templated class. The template
parameter of StoppointCallback would allow us to define a
std::unique_ptr<UserData> member where the user can hang any data they
want for the callback. That saves us from void pointers (which I find
very awkward).
template <class UserData>
class StoppointCallback {
private:
std::unique_ptr<UserData> m_data;
// ...
};
I also think that such a decision requires significant thought before we
pull that trigger. It does constitute a significant addition to the SB
api, tho I don't think it would break the SB api per se since we can
always use overloading and template specialization to keep the old
functionality. On the other hand, ABI compatibility may be a problem.
I don't know that much about SWIG or the needs of LLDB from a ABI
stability perspective, so please speak up if I'm suggesting something
crazy.
That said, it is somewhat difficult to keep the API consistent between
Breakpoint and Watchpoint using function pointers; the signature
associated with each will differ (e.g. Watchpoint should provide old and
new values of the subject variable, while that does not apply to
Breakpoint at all). I welcome any suggestions which allow us to avoid
logic duplication. My goto just happens to be templates here.
Doing even a subset of phase 2 would go a long ways toward fixing
problem 4.
Phase 3: Expanded Feature Set
-----------------------------
I would love to see these features:
* Watch variables from a function in every scope of that function OR in
only select invocations. Perhaps this already exists, but I can only
get it to watch from a particular scope.
* Better (read functional) remote GDB server break / watch. As has been
disused in other threads, this is not really easy but is very
desirable. For instance, I would very much like to see lldb play nice
with valgrind's vgdb server as it offers some amazing break / watch
functionality. As of today I need to do a great many hacks to make that
work properly.
* Finishpoints. This just seems obvious. (problem 6)
* GDB style tracepoints. This may be very difficult but it seems very
desirable.
Your thoughts are appreciated,
\author{Daniel Noland}
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
