> On Aug 29, 2014, at 9:29 AM, Zachary Turner <[email protected]> wrote:
>
> I actually think we are agreeing on many of the high-level issues. However,
> before I write anymore, I want to mention again that there would never be
> code that looks like this:
>
> #if HOST_THREAD == HostThreadMacOSX
> #endif
>
> instead it would like this:
>
> #if defined(__APPLE__)
> #endif
>
> Anyway, that's mostly irrelevant to the larger discussion, but just making
> sure it's clear because the first way looks pretty onerous.
Yes, as long as we agree neither should show up in generic code, this is mostly
theoretical point...
>
> To me, the Host layer is a place to put platform specific code. Some of that
> platform specific code is intended to abstract away the differences between
> platforms for generic lldb to use, as you mention. But some of that platform
> specific code is intended to support other platform specific code. Under the
> method I'm proposing, yes it would be possible to call a specific method from
> a generic place. But what I'm arguing is that it's already possible, and my
> patch actually improves the situation. For example, the following static
> methods remain in |Host| today:
>
> GetBundleDirectory
> ResolveExecutableInBundle
Actually these two shouldn't be in the Host interface, Greg discussed this a
couple of days ago. For lldb running on Windows, I should be able to say:
(lldb) target create MacApplication.app
(lldb) gdb-remote my_mac:12345
(lldb) run
or whatever. The complication is that we've used MacOS X specific calls to
figure out how a Mac binary is laid out, which we should probably do in a
generic way when we get around to it. But this is a minor point.
> SystemLog
> ThreadDetach
> ThreadCancel
There must be some way on Windows to tell a thread to exit at the next
cancellation point? Can you really not cancel a worker thread's operation?
> GetAuxvData
>
> Before my original HostInfo refactor, it also had these methods:
>
> GetUserID
> GetGroupID
> GetEffectiveUserID
> GetEffectiveGroupID
>
> Those methods were just as accessible to anyone writing generic code. They
> are *less* accessible now, because they are on HostInfoPosix. And this
> happened without needing to introduce any platform specific pre-processor
> defines into generic LLDB. There was maybe one exception, which Jason
> Molenda pointed out earlier, which was the GetOSBuildString. And that only
> happened because GetOSBuildString is not a generic concept! The design
> worked exactly as intended, exposing a place where code that was intended to
> be generic actually wasn't as generic as it thought it was. Everywhere else,
> the GetUserID, GetGroupID, etc methods were only being called from specific
> code (which is how it should work), but after my change this is now enforced
> by the compiler.
>
>
But again, if I'm writing code it seems like a real pain to have to answer the
question "is this an interface I can use in generic code" be:
For now you can if it exists in all the HostThreadXXX.h files, but of course if
somebody introduces another platform and decides that they don't want to
implement this function then you can't use it anymore and have to put
#if defined
guards around the usage. Instead, we should look at the host interfaces and
say there are three classes of things:
1) Things you must implement in order to port lldb to a host
These should be pure virtual methods in HostFeature.h
2) Things you can optionally implement, but there's a reasonable "couldn't do
that" fallback
These should be virtual methods in HostFeature.h
3) Things that are purely host specific.
These should be methods in HostFeatureMyHost.h, and can only be used in
HostOtherFeatureMyHost.h, but never in generic code.
This would make the job of folks working in generic code clear, and also make
it obvious to the next porter (OpenVMS, anyone?) what they have to do.
Jim
> On Fri, Aug 29, 2014 at 8:48 AM, <[email protected]> wrote:
> So maybe I'm missing something here. It seems to me that there are the
> specific details, say for instance of how the user/group for a process are
> specified. The details of how this is represented might be wildly different,
> but for the most part generic lldb doesn't care about that. However, it
> might want to know "Can the current user attach to this process" or "Create a
> debugserver that can attach to this process, doing whatever authentication
> you need to do." or just "print the owners of these processes..." These
> abstractions would seem to me the primary duties of the Host interface, which
> will back the Native debugger's tasks.
>
> Similarly, the main task of HostThread, so far as lldb is concerned, is
> Host::ThreadCreate, Host::ThreadCancel, Host::ThreadJoin. That's actually
> the hard part but most valuable part of the Host interface, as it seems to me.
>
> I'm concerned that this sort of functionality should be available to the
> generic parts of lldb in a way that isn't going to introduce any #if
> HOST_THREAD ==... in generic lldb code.
>
> Jim
>
>
> > On Aug 29, 2014, at 7:33 AM, Zachary Turner <[email protected]> wrote:
> >
> > I'm ok with this. It's possible I wasn't clear enough earlier in the
> > thread, but the typedef is already exactly how it works in the original
> > patch (look at HostThread.h in the patch), and the abstract base class I
> > agreed to early on in the thread when it was suggested by Jim.
> >
> > I actually view the buildbot breaks as a positive, I would much rather
> > catch these types of things at compile-time rather than at runtime. The
> > alternative proposed would make it easier to get specific methods into
> > generic code, because it hides the fact that it's happening due to the
> > specific method being a no-op on other platforms.
> >
> >
> > On Fri, Aug 29, 2014 at 6:55 AM, Jean-Daniel Dupas <[email protected]>
> > wrote:
> > 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
>
>
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
