My 2 cents, wouldn't it be possible to combine the best of both world: having a
formal protocol definition, and avoiding cast by using typedef ?
class HostThreadProtocol {
virtual required_method() = 0;
}
class WindowsThread : public HostThreadProtocol {
// Window implementation
}
class OSXThread : public HostThreadProtocol {
// OS X implementation
}
And then;
typedef WindowsThread HostThread;
or
typedef OSXThread HostThread;
Then, is someone want to implements Thread for a new host, it know exactly what
it has to implements, and we don't have to cast to specific class when needed.
The drawback I see is that you can accidentally call a specific method in
generic code, but the buildbot should catch that.
Jean-Daniel
Le 29 août 2014 à 07:14, Zachary Turner <[email protected]> a écrit :
> 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 <[email protected]> wrote:
>
>
>
> On Thu, Aug 28, 2014 at 9:42 PM, Jason Molenda <[email protected]> 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.
>
> _______________________________________________
> 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
