[Lldb-commits] [PATCH] Implements a HostThread class.
zturner at google.com
Thu Aug 28 22:14:48 PDT 2014
And FWIW, I have a patch locally which actually replaces all
lldb::thread_t's with HostThreads. No pre-processor definitions crept in.
ThreadJoin, Launch, and SetThreadName made up more than 95% of the
conversions, and those are all part of the common interface.
On Thu, Aug 28, 2014 at 10:05 PM, Zachary Turner <zturner at google.com> wrote:
> On Thu, Aug 28, 2014 at 9:42 PM, Jason Molenda <jmolenda at apple.com> wrote:
>> To take one small example, HostInfo::GetOSBuildString is defined for
>> three out of the four platforms. And so any generic code that might want
>> to call this has to do
>> if (IsHost())
>> #if !defined(__linux__)
>> return HostInfo::GetOSBuildString(s);
>> return false;
> But "os build string" and "os kernel version" are platform specific, by
> definition. I find it really hard to agree that we should just allow any
> old platform specific method to be exposed through a supposedly generic
> interface with different subsets of methods broken on different subsets of
> platforms. Then, you write some code to call a method like like
> HostInfo::GetRedApples(), and you have no idea under what situations it's
> even going to work. If a method isn't supported on your platform, you
> shouldn't be trying to call it. If you are, that's already a good
> indicator that you might have a layering problem.
>> A call into *any* HostInfo method may similarly need to be #ifdef
>> protected. And while it looks fine, to have #if !defined linux here, we
>> know that eventually someone will add a method that only makes sense on Mac
>> OS X (or so they think) and they'll do
>> #if defined (__APPLE__)
>> return HostInfo::GetRedApples();
>> And then Solaris 16 will be released and suddenly it can get red apples
>> too - a maintainer of the HostInfoSolaris class adds that method but then
>> has to go find all the ifdef blocks and make them defined __APPLE__ ||
>> I think we need a consistent error return API for functions in
>> HostInfo/HostThread to say "not available on this platform" and generic
>> code can use a different approach.
> Not all methods even return Error. And the cost of making every single
> method return Error is even higher, because nobody is ever going to check
> for this return value. And what do you do when you do get the return
> value? Call exit()? If the compiler catches this for you it doesn't have
> to be a runtime decision.
> Again, the number of places where this is going to happen are small. As
> Jim said, most generic LLDB code isn't and shouldn't be calling into Host.
> You can search through my HostInfo patch, for example, and find that I
> converted maybe 100 pre-processor definitions into about 2 or 3. I don't
> think this problem of #ifdef'ing will be as bad as you think it will be,
> and certainly it's better than before?
> HostInfo has another issue, btw, which is that it is a static class. So
> there's really no other way. There's no instance of it, just as there was
> no instance of Host. I could have gone and added methods for Windows jobs,
> SEH, and plenty of other highly windows specific concepts to Host.h and
> grew the class even more than it already was, and then stubbed out all the
> methods on every other platform, but is that really what you want? More
> likely to break the build, increases code size, increases build time, all
> for code that is never going to run on the platform. And since, by
> definition, the code should never run on the platform, why not find out at
> compile time if some code path calls it?
> The idea of "I'll just implement this method for my platform and stub it
> out for everyone else" just shifts the burden from a compile time burden to
> a run-time burden, which is almost always a worse proposition to have to
> deal with. In the end, the "public" interface of any given Host class is
> likely to stabilize and change very infrequently. The platform specific
> bits, on the other hand, are likely to evolve quite regularly. A design
> where people can work on the platform specific bits in isolation from every
> other platform without affecting anyone else is a huge win across the board.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the lldb-commits