[PATCH] D39111: Extensible LLVM RTTI

Wed Mar 18 14:41:01 PDT 2020

lhames added a comment.

In D39111#1929571 <https://reviews.llvm.org/D39111#1929571>, @dblaikie wrote:

> I think this bit amounts to "reservations because of Windows":
>
> "Sadly, [using a CRTP base to provide the ID] will (I suspect) run into the non-conforming implementation of static data members of class templates on MSVC between DLLs. Specifically, the address of ID will be different between two DLLs. =["

I think this was Chandler responding to your suggestion that the ID be moved into the CRTP base, rather than to this patch. :)

> "I'm actually a bit hesitant with this entire thing as a consequence. We need to really clearly document the hard constraints on how this CRTP base is used as a consequence of this (you cannot use this as part of class templates unless you explicitly instantiate all specializations of them)."

The "entire thing" presumably applies to this patch, but I think this just requires sufficient documentation. The rules are simple enough to convey:

1. Your class must provide a static char ID member (failure to do so will result in RTTI failures at runtime).

2. RTTIExtends should *only* be subclassed by classes, not class templates, due to Windows handling of ODR static data members.

If we wanted to preclude the problem with (1) we could switch to a traits class for supplying the ID, but we can't do much for (2) except document.

>> This certainly came up with the original design of the RTTI for Error: ErrorInfo originally defined the ID member for you, which Chandler pointed out is unsafe on Windows. We solved that in the ErrorInfo case by requiring you to define the ID member in your own class, and RTTIExtends takes the same approach.
> 
> The existence of the ErrorInfo doing something similar (could it be ported to use this infrastructure, then?)...

That's my plan: Replace ErrorInfo with RTTIExtends if this lands.

> This currently would vary by platform & some other things, such that it's a dynamic choice you want to check for?

Yep. The idea is that the client shouldn't have to replicate the JIT's internal logic to intuit the layer type, since that internal logic is subject to change and casting to the wrong type could (in the worst case) fail quietly but disastrously. Since the client may be using a repackaged JIT that could mix in its own layer types / setup I would consider this a dynamic choice on an effectively open hierarchy.

> I guess maybe that raises a thought: Is this about having an open hierarchy, where you can dyn_cast to specific pre-provided types, or necessarily to support dyn_casting to user-provided types too?

This is specifically to support a fully open hierarchy. A solution that allows dyn_cast between LLVM-provided types only (or any fixed set of types) would be insufficient.

> Same sort of question - I guess the LLVM version of this code would be closed, but users would provide their own kinds of MUs - to support those users using both their own custom ones and standard ones, do the users control the MUs, such that they could test for all the types of MUs they know they are using in their JIT. Yeah, that'd be a bit awkward to be "is it MyCustomMU3" ends up being "is it not an IRMU, not a <other LLVM-provided MU>, etc... then it must be a MyCustomMU of some kind, so then do a different pseudo RTTI test".

Yep. And what if the client is using a JIT built on top of ORC that has their own set of MU types? Then you can't even reason by elimination: Ok, it's not an LLVM MU, but is it one of mine, or one of the other library author's?

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D39111/new/

https://reviews.llvm.org/D39111