[PATCH] D48807: Add llvm::Any

[Zachary Turner via llvm-commits]

On Mon, Jul 2, 2018 at 1:32 PM David Blaikie via Phabricator <
reviews at reviews.llvm.org> wrote:

> dblaikie added a comment.
>
> In https://reviews.llvm.org/D48807#1149775, @zturner wrote:
>
> > I've wanted this for quite a long time.  My original motivation for this
> was back when i was working primarily in LLDB.  There's a lot of places
> where `void*` is passed around as a generic parameter to a user callback.
> So, for example, you say `DoSomethingAsynchronously(Callback, Param)` and
> when the thing is finished, it invokes `Callback(Param)`.  There are
> obviously better ways to do this if you're designing this from the
> beginning, but it's not always easy to retrofit that into an existing
> design.  So I think several places in LLDB could be cleaned up by replacing
> some `void*` with `Any`.  Note those places still exist today, so `Any`
> could be useful there right now.
>
>
> Would it be expensive to reimplement those APIs using stateful functors
> while providing the backwards compatibility with something like:
>
>   void register(void (*f)(void*), void *v) { register([f, v] { f(v); }); }
>
> Or is there something in the implementation (rather than the existing
> usage) that makes migrating difficult?

It’s been a while since I was reading close to any of this code so it’s
hard to say



>
> > More recently while designing a basic API for a tracing library, I've
> gravitated towards a design where the library generates events (represented
> by some structure) depending on what happens.  But since tracing is heavily
> platform dependent, there's only so much of this that you can abstract away
> into a common structure and there will be some additional pieces that only
> make sense on the particular platform.  So you call:
> >
> >   unique_ptr<EventBase> Event = llvm::trace::WaitForEvent();
> >   // Do something depending on what kind of event it is.
> >   llvm::trace::ContinueFromEvent(std::move(Event));
> >
> >
> > In order for every possible platform to implement this correctly, there
> may need to be some platform-specific state maintained between the call to
> Wait and the call to Continue.  Linux might need to pass the exact signal
> value and parameters.  Windows might need to pass the exact
> `EXCEPTION_RECORD` structure <
> https://docs.microsoft.com/en-us/windows/desktop/api/winnt/ns-winnt-_exception_record>
> that the OS filled out for us right before we return from
> `WaitForEvent()`.  It's useful to be able to put that into an `Any`, so
> that we have something like:
> >
> >   class EventBase {
> >     TraceEventType Type;
> >     uint64_t ThreadId;
> >     uint64_t ProcessId;
> >     Any PlatformPiece;
> >   };
> >
> >   class ThreadCreatedEvent : public EventBase {
> >     void *EntryPoint;
> >     void *TlsArea;
> >     // Etc.
> >   };
> >
> >   class BreakpointEvent : public EventBase {
> >     uint64_t Address;
> >   };
> >
> >
> > and so forth and so on.  When creating this struct before returning from
> `WaitForEvent()`, the library can set `PlatformPiece` to some structure
> that would contain any platform-specific state needed to properly
> continue.  I prefer not to use inheritance and `llvm::cast` here (e.g.
> `WindowsBreakpointEvent`, `PosixBreakpointEvent`, etc) because it just
> leads to inheritance hierarchy explosion and ultimately it's no more
> type-safe than `any` (someone still has to make an assumption about exactly
> what type it is).
>
> If this is host-platform specific, why would this need to be runtime
> variable at all? Wouldn't the PlatformPiece be hardcoded to some specific
> layout based on the host platform the compiler was built for/running on?


Not necessarily.  For example you may have an x64 build of llvm tracing a
32 bit process.  This can arise on Windows (wow64) as well as MachO
(universal binaries).  Maybe other platforms as well.  You could probably
narrow it to a variant, but we don’t have variant in llvm either and it’s
much harder to implement and im not sure it buys much.

Also, in a system where events are passed around and potentially held onto
by user code for a long time, it’s useful to have a generic “Tag” field
that the user can attach arbitrary data to. This is another potential use
case which I didn’t mention before.


 these 2-3 use cases aside, given that it’s in c++17, seems like filling as
many of the missing gaps as possible between our support libraries and
c++17 can only be a good thing
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180702/87278a81/attachment.html>