Page MenuHomePhabricator

Move Broadcaster+Listener+Event combo from Core into Utility
ClosedPublic

Authored by labath on Dec 6 2018, 3:04 AM.

Details

Summary

These are general purpose "utility" classes, whose functionality is not
debugger-specific in any way. As such, I believe they belong in the
Utility module.

This doesn't break any particular dependency (yet), but it reduces the
number of Core dependencies across the board.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Dec 6 2018, 3:04 AM
brucem added a subscriber: brucem.Dec 6 2018, 3:16 AM

I recognize that it might be dull, but should you re-sort the various include lists so that the now-in-Utility headers are listed with the other Utility headers?

labath updated this revision to Diff 176948.Dec 6 2018, 3:27 AM

That's a good point, thanks for reminding me.

Re-clang-formatting the changed lines.

LGTM modulo the unrelated clang-format changes. Feel free to commit them separately (even though the m_event_names fix looks a bit strange).

include/lldb/Utility/Broadcaster.h
551 ↗(On Diff #176948)

What was changed here? I assume clang-format fallout, so can we drop this?

source/Utility/Broadcaster.cpp
229 ↗(On Diff #176948)

Also unrelated.

239 ↗(On Diff #176948)

Also unrelated.

source/Utility/Event.cpp
139 ↗(On Diff #176948)

Also unrelated it seems.

teemperor accepted this revision.Dec 6 2018, 10:00 AM
This revision is now accepted and ready to land.Dec 6 2018, 10:00 AM
labath updated this revision to Diff 177212.Dec 7 2018, 7:24 AM
labath marked 3 inline comments as done.

Thanks for the review.

Yes the changes were done by clang-format, which even in the diff-only mode
doesn't know the difference between a moved file and a newly created one. I hope
this version successfully reverts those.

I'll keep this open a bit more to see if anyone else wants to say something. If
not I'll commit some time next week.

This revision was automatically updated to reflect the committed changes.