[Lldb-commits] [PATCH] Implements a HostThread class.

[Zachary Turner]

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);
>> #else
>>         return false;
>> #endif
>>
>
> 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();
>> #endif
>>
>> 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__ ||
>> __SOLARIS__.
>
>
>>
>> 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...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140828/88871804/attachment.html>