Add llvm::Any

[zturner]

Summary

This is a replacement for std::any until such time that we have C++17. While I generally agree it should not be used, and usually you want something like a variant, there are still occasions where it can be useful. One example is where a user wants to attach some user data to a structure that passes through library code and comes back around through a callback or similar mechanism. Another is when porting old-style C code that uses void*. I'm sure we can come up with plenty of other uses as well.

[dblaikie]

Hey Zach - what's your initial use case/motivation for this?

[zturner]

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.

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 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).

Hopefully that all makes sense.

[dblaikie]

In 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?

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 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?

Hopefully that all makes sense.

I think so - hopefully I'm replying in a way that does too.

[zturner]

Repeating my previous reply since it was through email and didn't make it onto Phab.

In D48807#1150029, @dblaikie wrote:

In 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 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 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

[zturner]

Also, one more area where Any makes code cleaner with my planned design is that there isn't really a 1-to-1 mapping between platform events and library events, because platforms have such different semantics.

For example, in Windows a single platform events corresponds to what on Linux is covered by the union of SIGTRAP, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGUSR, and other various signals. And those signals themselves are Linux specific, and may be different from how the same types of events are communicated on Mac, BSD, or some other platform.

To abstract all these platform differences away, it's nice for the library's view of events to be independent of any particular platform. You might have a BreakpointEvent, an ArithmeticErrorEvent, an IllegalInstructionEvent, an InvalidMemoryReferenceEvent, and so on and so forth. On Windows, however, the PlatformPiece would be of the exact same concrete type for all of these (and others). So if you had some Windows specific code, you'd end up doing something like:

unique_ptr<EventBase> E = waitForEvent();
EXCEPTION_RECORD *E;
switch (E->Type) {
case EventBase::Breakpoint:
  E = &(dyn_cast<BreakpointEvent>(E.get())->PlatformPiece);
  break;
case EventBase::ArithmeticError:
  E = &(dyn_cast<ArithmeticErrorEvent>(E.get())->PlatformPiece);
  break;
case EventBase::IllegalInstruction:
  E = &(dyn_cast<IllegalInstructionEvent>(E.get())->PlatformPiece);
  break;
case EventBase::InvalidMemoryReference:
  E = &(dyn_cast<InvalidMemoryReferenceEvent>(E.get())->PlatformPiece);
  break;
}

On the other hand, with Any you can store the PlatformPiece directly in EventBase, and this code becomes:

unique_ptr<EventBase> E = waitForEvent();
const EXCEPTION_RECORD *E;
switch (E->Type) {
case EventBase::Breakpoint:
case EventBase::ArithmeticError:
case EventBase::IllegalInstruction:
case EventBase::InvalidMemoryReference:
  E = llvm::any_cast<const EXCEPTION_RECORD*>(&E->PlatformPiece);
  break;
}

[timshen]

In D48807#1150258, @zturner wrote:

Also, one more area where Any makes code cleaner with my planned design is that there isn't really a 1-to-1 mapping between platform events and library events, because platforms have such different semantics.

For example, in Windows a single platform events corresponds to what on Linux is covered by the union of SIGTRAP, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGUSR, and other various signals. And those signals themselves are Linux specific, and may be different from how the same types of events are communicated on Mac, BSD, or some other platform.

To abstract all these platform differences away, it's nice for the library's view of events to be independent of any particular platform. You might have a BreakpointEvent, an ArithmeticErrorEvent, an IllegalInstructionEvent, an InvalidMemoryReferenceEvent, and so on and so forth. On Windows, however, the PlatformPiece would be of the exact same concrete type for all of these (and others). So if you had some Windows specific code, you'd end up doing something like:

unique_ptr<EventBase> E = waitForEvent();
EXCEPTION_RECORD *E;
switch (E->Type) {
case EventBase::Breakpoint:
  E = &(dyn_cast<BreakpointEvent>(E.get())->PlatformPiece);
  break;
case EventBase::ArithmeticError:
  E = &(dyn_cast<ArithmeticErrorEvent>(E.get())->PlatformPiece);
  break;
case EventBase::IllegalInstruction:
  E = &(dyn_cast<IllegalInstructionEvent>(E.get())->PlatformPiece);
  break;
case EventBase::InvalidMemoryReference:
  E = &(dyn_cast<InvalidMemoryReferenceEvent>(E.get())->PlatformPiece);
  break;
}

On the other hand, with Any you can store the PlatformPiece directly in EventBase, and this code becomes:

unique_ptr<EventBase> E = waitForEvent();
const EXCEPTION_RECORD *E;
switch (E->Type) {
case EventBase::Breakpoint:
case EventBase::ArithmeticError:
case EventBase::IllegalInstruction:
case EventBase::InvalidMemoryReference:
  E = llvm::any_cast<const EXCEPTION_RECORD*>(&E->PlatformPiece);
  break;
}

Aren't virtual functions exactly for homogeneous operations over sub-classes?

unique_ptr<EventBase> Event = waitForEvent();
const EXCEPTION_RECORD *E;
switch (Event->Type) {
case EventBase::Breakpoint:
case EventBase::ArithmeticError:
case EventBase::IllegalInstruction:
case EventBase::InvalidMemoryReference:
  E = Event->getPlatformPiece();  // getPlatformPiece() is a virtual function in EventBase.
  break;
}

[zturner]

In D48807#1151011, @timshen wrote:

Aren't virtual functions exactly for homogeneous operations over sub-classes?

unique_ptr<EventBase> Event = waitForEvent();
const EXCEPTION_RECORD *E;
switch (Event->Type) {
case EventBase::Breakpoint:
case EventBase::ArithmeticError:
case EventBase::IllegalInstruction:
case EventBase::InvalidMemoryReference:
  E = Event->getPlatformPiece();  // getPlatformPiece() is a virtual function in EventBase.
  break;
}

And what type does it return?

[zturner]

In D48807#1151012, @zturner wrote:

In D48807#1151011, @timshen wrote:

Aren't virtual functions exactly for homogeneous operations over sub-classes?

unique_ptr<EventBase> Event = waitForEvent();
const EXCEPTION_RECORD *E;
switch (Event->Type) {
case EventBase::Breakpoint:
case EventBase::ArithmeticError:
case EventBase::IllegalInstruction:
case EventBase::InvalidMemoryReference:
  E = Event->getPlatformPiece();  // getPlatformPiece() is a virtual function in EventBase.
  break;
}

And what type does it return?

To elaborate, EXCEPTION_RECORD here is a platform specific type. It will fail to compile on any platform except Windows. So we can't expose that through a platform-agnostic interface. We could wrap it in a base class whose sole purpose is to hide this, such as:

struct PlatformPieceBase {
  virtual ~PlatformPieceBase() {}
};

struct Win32ExceptionRecordPlatformPiece : public PlatformPiece {
  EXCEPTION_RECORD E;
};

and then re-write the line in the switch as:

E = llvm::cast<Win32ExceptionRecordPlatformPiece>(Event->getPlatformPiece())->E;

But now you need to build an entire inheritance hierarchy just to wrap one structure which you already have an instance of from the OS. It seems like a lot of trouble to avoid using Any, and the "building an inheritance hierarchy to wrap..." is ultimately exactly what the implementation of Any does anyway.

[zturner]

To re-iterate a couple of other points mentioned earlier:

1. For a given event type and host platform there would usually (but not always) be one type per platform. The not always is important, because it means a runtime discovery process is necessary in some cases.

2. There are multiple event types, so if you did have a virtual function accessible from the base class, it would need to decide on a single common type to return. However, since there isn't a common type, you're ultimately just re-inventing Any without using Any by artificially injecting a base class that doesn't really do anything except satisfy the type system.

3. Any is in C++17, which LLVM will eventually adopt

[zturner]

Ping. Anyone have any comments on the implementation?

[dblaikie]

In D48807#1150245, @zturner wrote:

Repeating my previous reply since it was through email and didn't make it onto Phab.

In D48807#1150029, @dblaikie wrote:

In 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 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 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.

Ah, good point.

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.

I'd say variant buys a fair bit in terms of type safety/self-documenting code. I worry about both variant and any in terms of what they do to type safety/contracts between different pieces of code, hence my pushback on adding these kinds of constructs to LLVM.

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.

I'm still having a little trouble understanding what code is going to consume these events (& how's it going to know/support the different kinds of things in them) given how arbitrary (no common interface, etc) they may be?

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

If we were working with C++17 I'd push back in the same way against adding usage of these types to LLVM code for the same concerns (type safety, self documenting code, etc). Much like LLVM not using exceptions, iostreams, etc - not everything in the language is suitable for every codebase.

llvm/include/llvm/ADT/Any.h

25 ↗

(On Diff #153639)

I think an out of line definition of Id is necessary here, maybe?

(also you can potentially do static const void *Id; + const void *TypeId::Id = &TypeId::Id; basically)

& I'd probably make TypeId a private implementation detail of 'Any'?

29 ↗

(On Diff #153639)

= default

30 ↗

(On Diff #153639)

Could probably add a cloneable CRTP helper some day - no doubt there are other hierarchies that could use that.

34–35 ↗

(On Diff #153639)

Feels like you maybe might as well use the 'struct' keyword here too - since everything's public, except the deleted operators (which don't particularly benefit from being private anyway - so probably make them public too)

54 ↗

(On Diff #153639)

= default

76–85 ↗

(On Diff #153639)

You could collapse these into one operator that takes Any by value if you like.

Any &operator=(Any Other) {
  Storage = std::move(Other.Storage);
  return *this;
}

91–93 ↗

(On Diff #153639)

Might as well match std::any and have these be called has_value() and reset()?

108 ↗

(On Diff #153639)

Why add the 'const' here - if cv qualifiers are being removed anyway? Maybe I'm missing something about how std::decay/other parts of this are working?

(also std::decay says it removes cv qualifiers - so is it necessary to then explicitly remove them after that?)

114–135 ↗

(On Diff #153639)

Implement these in terms of the pointer variants, perhaps? (to reduce duplication)

133–134 ↗

(On Diff #153639)

Is this the right behavior according to the standard version of std::any? (should the passed in Any be left in the empty state, or in the non-empty state with a moved-from value? It looks like maybe the latter)

llvm/unittests/ADT/AnyTest.cpp

120–122 ↗

(On Diff #153639)

Maybe create a named local variable for the pointer here - to demonstrate that this code is valid:

int *i = llvm::any_cast<int>(&A);
*i = 42;
...

The test as-is would pass even if any_cast returned a proxy object, which I would guess we don't want to allow (& instead want to allow/guarantee that users can write code that keeps pointers, etc).

[zturner]

In D48807#1156823, @dblaikie wrote:

In D48807#1150245, @zturner wrote:

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.

I'd say variant buys a fair bit in terms of type safety/self-documenting code. I worry about both variant and any in terms of what they do to type safety/contracts between different pieces of code, hence my pushback on adding these kinds of constructs to LLVM.

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.

I'm still having a little trouble understanding what code is going to consume these events (& how's it going to know/support the different kinds of things in them) given how arbitrary (no common interface, etc) they may be?

There are certain things that are common to all platforms, and certain things that aren't. For example, if your process hits a trap instruction like an int 3, then no matter what platform you're on, there's going to be an address at which it occurred, a process id and thread id in the tracee, and a register context. On Windows, you'll have an EXCEPTION_DEBUG_INFO structure (https://msdn.microsoft.com/en-us/library/windows/desktop/ms679326(v=vs.85).aspx) which contains various other OS-specific bits of information that could be useful to platform specific code. I'm not sure what other things you might have on Linux, OSX, or BSD, but it's safe to assume there's something. If not specifically for a trap, for some kind of event (For example, when you call ptrace and it returns, it might have returned for many different reasons, and each reason means different info is available to query).

So the point of all this is: When a platform-specific event occurs, there is platform specific data associated with that event, only some of which can be factored out into a common interface, but all of which can be useful in certain scenarios.

So we give back the user an event object that has as wide of a common interface as possible, but still doesn't prevent access to all of the bits. The consumer of the common interface will be common code. Code that wants to decide what to do after hitting a breakpoint probably doesn't need all this platform specific information. It can just stop, look up line and symbol information based on the address, and report it to the user. But in a system like this which has deep hooks into the OS, it will be very frequent that the code handling an event needs to do something completely different depending on the platform. So we branch out into platform specific code, and in that platform specific code we know how to consume the platform specific event data.

[dblaikie]

So we give back the user an event object that has as wide of a common interface as possible, but still doesn't prevent access to all of the bits. The consumer of the common interface will be common code. Code that wants to decide what to do after hitting a breakpoint probably doesn't need all this platform specific information. It can just stop, look up line and symbol information based on the address, and report it to the user. But in a system like this which has deep hooks into the OS, it will be very frequent that the code handling an event needs to do something completely different depending on the platform. So we branch out into platform specific code, and in that platform specific code we know how to consume the platform specific event data.

Sounds pretty much the same as what LLVM's codebase would use polymorphism for (& dyn_cast, dyn_cast if chains, or switch over the type+cast, etc) since its inception. Some common API & some specific stuff that certain consumers care about. Doesn't necessarily mean it's the right way to do it - but there's certainly a lot of precedent (admittedly implemented before something like Any was as commonly thought about in C++, maybe).

Wouldn't mind hearing some other perspectives in here - I'm still a bit reticent, but happy enough to sign off (once the technical feedback is addressed) if you're really feeling it.

[zturner]

In D48807#1157663, @dblaikie wrote:

So we give back the user an event object that has as wide of a common interface as possible, but still doesn't prevent access to all of the bits. The consumer of the common interface will be common code. Code that wants to decide what to do after hitting a breakpoint probably doesn't need all this platform specific information. It can just stop, look up line and symbol information based on the address, and report it to the user. But in a system like this which has deep hooks into the OS, it will be very frequent that the code handling an event needs to do something completely different depending on the platform. So we branch out into platform specific code, and in that platform specific code we know how to consume the platform specific event data.

Sounds pretty much the same as what LLVM's codebase would use polymorphism for (& dyn_cast, dyn_cast if chains, or switch over the type+cast, etc) since its inception. Some common API & some specific stuff that certain consumers care about. Doesn't necessarily mean it's the right way to do it - but there's certainly a lot of precedent (admittedly implemented before something like Any was as commonly thought about in C++, maybe).

Wouldn't mind hearing some other perspectives in here - I'm still a bit reticent, but happy enough to sign off (once the technical feedback is addressed) if you're really feeling it.

Yes, the dyn_cast stuff is a good analogy. And it would be theoretically possible to do what I need with dyn_cast and a base class. The reason I'm hesitant to go that route though and prefer any over this approach is because it would really explode the inheritance hierarchy. For every event type you would need at least 1 subclass per platform. Just for a simple trap event, you'd need LinuxTrapEvent, BSDTrapEvent, Win32TrapEvent, Win64TrapEvent, WinWow6432TrapEvent. And ultimately each of these would just would probably just contain 1 or 2 trivially copyable fields that you got directly from an OS syscall. So the only purpose of the subclass would be to wrap some structure, not really to provide any actual functionality. At that point you're already skirting pretty close to Any anyway

llvm/include/llvm/ADT/Any.h

108 ↗

(On Diff #153639)

I need to think about this some. I "borrowed" this line from MSVC's implementation of std::any, and without it I was getting mysterious errors. But it wouldn't hurt for me to understand exactly why.

[zturner]

Updated version based on several suggestions from @dblaikie

[zturner]

ping

[dblaikie]

llvm/unittests/ADT/AnyTest.cpp

114–115 ↗

(On Diff #155262)

Is there a test (didn't spot it) that demonstrates that the result of llvm::any_cast on an rvalue Any is the value moved-from the the Any, and that the Any contains the original move-from?

[zturner]

Added a test suggested by @dblaikie. Also reworked the enable_if condition on the copy-constructor based on comments from @rsmith that the previous check was incorrect for reasons that are still beyond my comprehension. In doing this, I had to add conjunction and negate -- which are part of C++17 -- to STLExtras.

llvm/unittests/ADT/AnyTest.cpp

114–115 ↗

(On Diff #155262)

Is the newly added test in line with what you had in mind?

[dblaikie]

llvm/unittests/ADT/AnyTest.cpp

114–115 ↗

(On Diff #155262)

Yeah, looks good!

llvm/trunk/include/llvm/ADT/Any.h

+//===- Any.h - Generic type erased holder of any type -----------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file provides Any, a non-template class modeled in the spirit of
+//  std::any.  The idea is to provide a type-safe replacement for C's void*.
+//  It can hold a value of any copy-constructible copy-assignable type
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ADT_ANY_H
+#define LLVM_ADT_ANY_H
+
+#include "llvm/ADT/STLExtras.h"
+
+#include <cassert>
+#include <memory>
+#include <type_traits>
+
+namespace llvm {
+
+class Any {
+  template <typename T> struct TypeId { static const char Id = 0; };
+
+  struct StorageBase {
+    virtual ~StorageBase() = default;
+    virtual std::unique_ptr<StorageBase> clone() const = 0;
+    virtual const void *id() const = 0;
+  };
+
+  template <typename T> struct StorageImpl : public StorageBase {
+    explicit StorageImpl(const T &Value) : Value(Value) {}
+
+    explicit StorageImpl(T &&Value) : Value(std::move(Value)) {}
+
+    std::unique_ptr<StorageBase> clone() const override {
+      return llvm::make_unique<StorageImpl<T>>(Value);
+    }
+
+    const void *id() const override { return &TypeId<T>::Id; }
+
+    T Value;
+
+  private:
+    StorageImpl &operator=(const StorageImpl &Other) = delete;
+    StorageImpl(const StorageImpl &Other) = delete;
+  };
+
+public:
+  Any() = default;
+
+  Any(const Any &Other)
+      : Storage(Other.Storage ? Other.Storage->clone() : nullptr) {}
+
+  // When T is Any or T is not copy-constructible we need to explicitly disable
+  // the forwarding constructor so that the copy constructor gets selected
+  // instead.
+  template <
+      typename T,
+      typename std::enable_if<
+          llvm::conjunction<
+              llvm::negation<std::is_same<typename std::decay<T>::type, Any>>,
+              std::is_copy_constructible<typename std::decay<T>::type>>::value,
+          int>::type = 0>
+  Any(T &&Value) {
+    using U = typename std::decay<T>::type;
+    Storage = llvm::make_unique<StorageImpl<U>>(std::forward<T>(Value));
+  }
+
+  Any(Any &&Other) : Storage(std::move(Other.Storage)) {}
+
+  Any &swap(Any &Other) {
+    std::swap(Storage, Other.Storage);
+    return *this;
+  }
+
+  Any &operator=(Any Other) {
+    Storage = std::move(Other.Storage);
+    return *this;
+  }
+
+  bool hasValue() const { return !!Storage; }
+
+  void reset() { Storage.reset(); }
+
+private:
+  template <class T> friend T any_cast(const Any &Value);
+  template <class T> friend T any_cast(Any &Value);
+  template <class T> friend T any_cast(Any &&Value);
+  template <class T> friend const T *any_cast(const Any *Value);
+  template <class T> friend T *any_cast(Any *Value);
+  template <typename T> friend bool any_isa(const Any &Value);
+
+  std::unique_ptr<StorageBase> Storage;
+};
+
+template <typename T> bool any_isa(const Any &Value) {
+  if (!Value.Storage)
+    return false;
+  using U =
+      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+  return Value.Storage->id() == &Any::TypeId<U>::Id;
+}
+
+template <class T> T any_cast(const Any &Value) {
+  using U =
+      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+  return static_cast<T>(*any_cast<U>(&operand));
+}
+
+template <class T> T any_cast(Any &Value) {
+  using U =
+      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+  return static_cast<T>(*any_cast<U>(&Value));
+}
+
+template <class T> T any_cast(Any &&Value) {
+  using U =
+      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+  return static_cast<T>(std::move(*any_cast<U>(&Value)));
+}
+
+template <class T> const T *any_cast(const Any *Value) {
+  using U =
+      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+  assert(Value && any_isa<T>(*Value) && "Bad any cast!");
+  if (!Value || !any_isa<U>(*Value))
+    return nullptr;
+  return &static_cast<Any::StorageImpl<U> &>(*Value->Storage).Value;
+}
+
+template <class T> T *any_cast(Any *Value) {
+  using U = typename std::decay<T>::type;
+  assert(Value && any_isa<U>(*Value) && "Bad any cast!");
+  if (!Value || !any_isa<U>(*Value))
+    return nullptr;
+  return &static_cast<Any::StorageImpl<U> &>(*Value->Storage).Value;
+}
+
+} // end namespace llvm
+
+#endif // LLVM_ADT_ANY_H

llvm/trunk/include/llvm/ADT/STLExtras.h

 
 template <typename RangeT>
 using ValueOfRange = typename std::remove_reference<decltype(
     *std::begin(std::declval<RangeT &>()))>::type;
 
 } // end namespace detail
 
 //===----------------------------------------------------------------------===//
+//     Extra additions to <type_traits>
+//===----------------------------------------------------------------------===//
+
+template <typename T> struct negation : std::bool_constant<!bool(T::value)> {};
+
+template <typename...> struct conjunction : std::true_type {};
+template <typename B1> struct conjunction<B1> : B1 {};
+template <typename B1, typename... Bn>
+struct conjunction<B1, Bn...>
+    : std::conditional<bool(B1::value), conjunction<Bn...>, B1>::type {};
+
+//===----------------------------------------------------------------------===//
 //     Extra additions to <functional>
 //===----------------------------------------------------------------------===//
 
 template <class Ty> struct identity {
   using argument_type = Ty;
 
   Ty &operator()(Ty &self) const {
     return self;

llvm/trunk/unittests/ADT/AnyTest.cpp

+//===- llvm/unittest/Support/AnyTest.cpp - Any tests ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/Any.h"
+#include "gtest/gtest.h"
+#include <cstdlib>
+
+using namespace llvm;
+
+namespace {
+
+// Make sure we can construct, copy-construct, move-construct, and assign Any's.
+TEST(AnyTest, ConstructionAndAssignment) {
+  llvm::Any A;
+  llvm::Any B{7};
+  llvm::Any C{8};
+  llvm::Any D{"hello"};
+  llvm::Any E{3.7};
+
+  // An empty Any is not anything.
+  EXPECT_FALSE(A.hasValue());
+  EXPECT_FALSE(any_isa<int>(A));
+
+  // An int is an int but not something else.
+  EXPECT_TRUE(B.hasValue());
+  EXPECT_TRUE(any_isa<int>(B));
+  EXPECT_FALSE(any_isa<float>(B));
+
+  EXPECT_TRUE(C.hasValue());
+  EXPECT_TRUE(any_isa<int>(C));
+
+  // A const char * is a const char * but not an int.
+  EXPECT_TRUE(D.hasValue());
+  EXPECT_TRUE(any_isa<const char *>(D));
+  EXPECT_FALSE(any_isa<int>(D));
+
+  // A double is a double but not a float.
+  EXPECT_TRUE(E.hasValue());
+  EXPECT_TRUE(any_isa<double>(E));
+  EXPECT_FALSE(any_isa<float>(E));
+
+  // After copy constructing from an int, the new item and old item are both
+  // ints.
+  llvm::Any F(B);
+  EXPECT_TRUE(B.hasValue());
+  EXPECT_TRUE(F.hasValue());
+  EXPECT_TRUE(any_isa<int>(F));
+  EXPECT_TRUE(any_isa<int>(B));
+
+  // After move constructing from an int, the new item is an int and the old one
+  // isn't.
+  llvm::Any G(std::move(C));
+  EXPECT_FALSE(C.hasValue());
+  EXPECT_TRUE(G.hasValue());
+  EXPECT_TRUE(any_isa<int>(G));
+  EXPECT_FALSE(any_isa<int>(C));
+
+  // After copy-assigning from an int, the new item and old item are both ints.
+  A = F;
+  EXPECT_TRUE(A.hasValue());
+  EXPECT_TRUE(F.hasValue());
+  EXPECT_TRUE(any_isa<int>(A));
+  EXPECT_TRUE(any_isa<int>(F));
+
+  // After move-assigning from an int, the new item and old item are both ints.
+  B = std::move(G);
+  EXPECT_TRUE(B.hasValue());
+  EXPECT_FALSE(G.hasValue());
+  EXPECT_TRUE(any_isa<int>(B));
+  EXPECT_FALSE(any_isa<int>(G));
+}
+
+TEST(AnyTest, GoodAnyCast) {
+  llvm::Any A;
+  llvm::Any B{7};
+  llvm::Any C{8};
+  llvm::Any D{"hello"};
+  llvm::Any E{'x'};
+
+  // Check each value twice to make sure it isn't damaged by the cast.
+  EXPECT_EQ(7, llvm::any_cast<int>(B));
+  EXPECT_EQ(7, llvm::any_cast<int>(B));
+
+  EXPECT_STREQ("hello", llvm::any_cast<const char *>(D));
+  EXPECT_STREQ("hello", llvm::any_cast<const char *>(D));
+
+  EXPECT_EQ('x', llvm::any_cast<char>(E));
+  EXPECT_EQ('x', llvm::any_cast<char>(E));
+
+  llvm::Any F(B);
+  EXPECT_EQ(7, llvm::any_cast<int>(F));
+  EXPECT_EQ(7, llvm::any_cast<int>(F));
+
+  llvm::Any G(std::move(C));
+  EXPECT_EQ(8, llvm::any_cast<int>(G));
+  EXPECT_EQ(8, llvm::any_cast<int>(G));
+
+  A = F;
+  EXPECT_EQ(7, llvm::any_cast<int>(A));
+  EXPECT_EQ(7, llvm::any_cast<int>(A));
+
+  E = std::move(G);
+  EXPECT_EQ(8, llvm::any_cast<int>(E));
+  EXPECT_EQ(8, llvm::any_cast<int>(E));
+
+  // Make sure we can any_cast from an rvalue and that it's properly destroyed
+  // in the process.
+  EXPECT_EQ(8, llvm::any_cast<int>(std::move(E)));
+  EXPECT_TRUE(E.hasValue());
+
+  // Make sure moving from pointers gives back pointers, and that we can modify
+  // the underlying value through those pointers.
+  EXPECT_EQ(7, *llvm::any_cast<int>(&A));
+  int *N = llvm::any_cast<int>(&A);
+  *N = 42;
+  EXPECT_EQ(42, llvm::any_cast<int>(A));
+
+  // Make sure that we can any_cast to a reference and this is considered a good
+  // cast, resulting in an lvalue which can be modified.
+  llvm::any_cast<int &>(A) = 43;
+  EXPECT_EQ(43, llvm::any_cast<int>(A));
+}
+
+TEST(AnyTest, CopiesAndMoves) {
+  struct TestType {
+    TestType() = default;
+    TestType(const TestType &Other)
+        : Copies(Other.Copies + 1), Moves(Other.Moves) {}
+    TestType(TestType &&Other) : Copies(Other.Copies), Moves(Other.Moves + 1) {}
+    int Copies = 0;
+    int Moves = 0;
+  };
+
+  // One move to get TestType into the Any, and one move on the cast.
+  TestType T1 = llvm::any_cast<TestType>(Any{TestType()});
+  EXPECT_EQ(0, T1.Copies);
+  EXPECT_EQ(2, T1.Moves);
+
+  // One move to get TestType into the Any, and one copy on the cast.
+  Any A{TestType()};
+  TestType T2 = llvm::any_cast<TestType>(A);
+  EXPECT_EQ(1, T2.Copies);
+  EXPECT_EQ(1, T2.Moves);
+
+  // One move to get TestType into the Any, and one move on the cast.
+  TestType T3 = llvm::any_cast<TestType>(std::move(A));
+  EXPECT_EQ(0, T3.Copies);
+  EXPECT_EQ(2, T3.Moves);
+}
+
+TEST(AnyTest, BadAnyCast) {
+  llvm::Any A;
+  llvm::Any B{7};
+  llvm::Any C{"hello"};
+  llvm::Any D{'x'};
+
+  EXPECT_DEBUG_DEATH(llvm::any_cast<int>(A), "");
+
+  EXPECT_DEBUG_DEATH(llvm::any_cast<float>(B), "");
+  EXPECT_DEBUG_DEATH(llvm::any_cast<int *>(B), "");
+
+  EXPECT_DEBUG_DEATH(llvm::any_cast<std::string>(C), "");
+
+  EXPECT_DEBUG_DEATH(llvm::any_cast<unsigned char>(D), "");
+}
+
+} // anonymous namespace

llvm/trunk/unittests/ADT/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_llvm_unittest(ADTTests
+  AnyTest.cpp
   APFloatTest.cpp
   APIntTest.cpp
   APSIntTest.cpp
   ArrayRefTest.cpp
   BitmaskEnumTest.cpp
   BitVectorTest.cpp
   BreadthFirstIteratorTest.cpp
   BumpPtrListTest.cpp