<div dir="ltr">I toiled over this question for a while, but ultimately decided against a base class with virtual methods because there will only ever be 1 type of HostThread per host platform. For example, on MacOSX, the HostThread type is HostThreadMacOSX. There's no need for polymorphism or the overhead that comes with virtual dispatch. Furthermore it complicates things. Suppose you have a HostThreadBase, and you pass a pointer to this into some mac specific class. Now it has to cast, which is ugly.<div>
<div><br></div><div>It's true, there is a level of awkwardness that comes with collecting together the intersection of all the derived types. However, it's something that we could do, for example, inside of HostThread.h and document the common interface there in a comment. In the grand scheme of what I'm envisioning, you will need to use Host code from outside of Host (obviously), but this will be the exception, at well-defined entry points into the host layer, with a vast majority of host code being used from within itself. This internal code would have access to the full interface since it's already under the assumption that it's compiling for the specific platform.</div>
</div><div><br></div><div>Let me know if it's not clear.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 28, 2014 at 4:40 PM, <span dir="ltr"><<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Maybe I need to wait till I see you use this, but I'm kind of surprised that the HostThread implementation has no base HostThread class here. There is generic code in lldb that will need or want to use HostThreads. For instance we have to create threads in lldb (e.g. Process has to create the thread that manages the internal state machine, and ProcessGDBRemote the reading thread) and we like to name the threads we create so that debugging lldb and reading Crash logs is easier, etc.<br>
<br>
As you have it written, how do I know which HostThread methods it is safe to call in generic code? And how do we make sure we have a common interface to these methods across all implementations. Am I just supposed to read all the HostThreadXXX.h and use only the intersection of their public methods. That seems awkward.<br>
<br>
I expected to have a HostThread.h that declared as virtual methods all the thread operations you must provide to be a host onto which lldb can be ported, and the interface you must provide. Maybe we could make a distinction between necessary (like Create) and useful (like Name) where the former has to work for lldb to function, and the latter can be no-op's. Then the HostThreadXXX classes would derive from this, and add whatever native methods they offered, but those methods could only be called from HostXXX code. If you started needing to call HostThreadXXX methods in generic code, then you were doing something wrong.<br>
<br>
Jim<br>
<div><div class="h5"><br>
<br>
> On Aug 28, 2014, at 2:51 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
><br>
> Hi majnemer, rnk, emaste, tfiala,<br>
><br>
> This class implements a host thread for the different platforms. As with HostProcess, no code has been updated to use HostThread yet, as this will clutter up the code review unnecessarily. However, the methods of HostThread generally map to methods of Host, so the conversion should be mostly mechanical. Because the actual conversion of client code has not been updated to use HostThread yet, it means the code in HostThread is largely duplicated. This duplication will be removed after the transition takes place.<br>
><br>
> I'm not super good at Linux, MacOSX, or FreeBSD, so I've included various people as reviewers in hopes of catching anything I may have implemented incorrectly. But don't feel obligated to review everything.<br>
><br>
> I've compiled and tested these changes on Mac, Linux, and Windows. (note, however, since nobody has been updated to use HostThread yet, there's little to actually test).<br>
><br>
> <a href="http://reviews.llvm.org/D5110" target="_blank">http://reviews.llvm.org/D5110</a><br>
><br>
> Files:<br>
> include/lldb/Core/DataBuffer.h<br>
> include/lldb/Host/HostThread.h<br>
> include/lldb/Host/freebsd/HostThreadFreeBSD.h<br>
> include/lldb/Host/linux/HostThreadLinux.h<br>
> include/lldb/Host/macosx/HostThreadMacOSX.h<br>
> include/lldb/Host/posix/HostThreadPosix.h<br>
> include/lldb/Host/windows/HostThreadWindows.h<br>
> include/lldb/lldb-types.h<br>
> lldb.xcodeproj/project.pbxproj<br>
> source/Host/CMakeLists.txt<br>
> source/Host/common/Host.cpp<br>
> source/Host/freebsd/Host.cpp<br>
> source/Host/freebsd/HostThreadFreeBSD.cpp<br>
> source/Host/linux/Host.cpp<br>
> source/Host/linux/HostThreadLinux.cpp<br>
> source/Host/macosx/Host.mm<br>
> source/Host/macosx/HostThreadMacOSX.cpp<br>
> source/Host/posix/HostThreadPosix.cpp<br>
> source/Host/windows/HostThreadWindows.cpp<br>
</div></div>> <D5110.13058.patch>_______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br>
</blockquote></div><br></div>