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.
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. 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. Jim > On Aug 28, 2014, at 2:51 PM, Zachary Turner <[email protected]> wrote: > > Hi majnemer, rnk, emaste, tfiala, > > 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. > > 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. > > 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). > > http://reviews.llvm.org/D5110 > > Files: > include/lldb/Core/DataBuffer.h > include/lldb/Host/HostThread.h > include/lldb/Host/freebsd/HostThreadFreeBSD.h > include/lldb/Host/linux/HostThreadLinux.h > include/lldb/Host/macosx/HostThreadMacOSX.h > include/lldb/Host/posix/HostThreadPosix.h > include/lldb/Host/windows/HostThreadWindows.h > include/lldb/lldb-types.h > lldb.xcodeproj/project.pbxproj > source/Host/CMakeLists.txt > source/Host/common/Host.cpp > source/Host/freebsd/Host.cpp > source/Host/freebsd/HostThreadFreeBSD.cpp > source/Host/linux/Host.cpp > source/Host/linux/HostThreadLinux.cpp > source/Host/macosx/Host.mm > source/Host/macosx/HostThreadMacOSX.cpp > source/Host/posix/HostThreadPosix.cpp > source/Host/windows/HostThreadWindows.cpp > <D5110.13058.patch>_______________________________________________ > lldb-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
