This is an archive of the discontinued LLVM Phabricator instance.

Move Predicate.h from Host to Utility
ClosedPublic

Authored by labath on Aug 7 2018, 5:58 AM.

Details

Summary

This class was initially in Host because its implementation used to be
very OS-specific. However, with C++11, it has become a very simple
std::condition_variable wrapper, with no host-specific code.

It is also a general purpose utility class, so it makes sense for it to
live in a place where it can be used by everyone.

This has no effect on the layering right now, but it enables me to later
move the Listener+Broadcaster+Event combo to a lower layer, which is
important, as these are used in a lot of places (notably for launching a
process in Host code).

Diff Detail

Repository
rLLDB LLDB

Event Timeline

labath created this revision.Aug 7 2018, 5:58 AM

Hurray for more layering improvements!

It looks like Raphael recently committed a modulemap for compiling the LLDB headers into C++ modules (include/lldb/module.modulemap) that has a reference to Predicate.h in in Host. You should probably modify that as well.

labath updated this revision to Diff 159649.Aug 8 2018, 1:30 AM

Updating the module map too (thanks).

zturner accepted this revision.Aug 8 2018, 8:33 AM

lgtm, although Predicate is simple enough that I wonder if one day we should try to just delete it entirely.

This revision is now accepted and ready to land.Aug 8 2018, 8:33 AM

Is this blocked by something? If you're busy @labath I can commit this and watch the bots.

Not blocked on anything, just haven't gotten around to committing it yet. Feel free to land it for me.

Also, to answer Zachary's question:

A lot of the existing uses of Predicate class could be replaced by std::future (often future<void>). The one in the Event class is in this category. However, there are some uses (these tend to be hacks of various magnitudes), which are not as easy to replace.

This revision was automatically updated to reflect the committed changes.
unittests/Core/BroadcasterTest.cpp