This is an archive of the discontinued LLVM Phabricator instance.

[LanguageRuntime] Introdce LLVM-style casts
ClosedPublic

Authored by xiaobai on Jun 5 2019, 3:05 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

xiaobai created this revision.Jun 5 2019, 3:05 PM

Implementing classof via an enum is the simplest and most common solution, but it is not the only thing possible. The central enum thingy is fine when all the subclasses are also defined centrally, but for a more distributed scenario like this one, it begins to smell, as now the LanguageRuntime class suddenly needs to be aware of all of its subclasses.

A way to implement that without the all-encompassing enum would be via something like:

struct Runtime: {
virtual bool isA(const void *ClassID) const { return ClassID == &ID; }
static char ID;
};

...

struct DerivedRuntime: ParentRuntime {
bool isA(const void *ClassID) const override { return ClassID == &ID || ParentRuntime::isA(ClassID); }

static bool classof(const Runtime *R) { return R->isA(&ID); }
static char ID;
};

It takes a while to wrap your head around this, but what it basically does is use pointer equality instead of enum comparisons. Then, if you have a multi level hierarchy (like we have here), it uses the virtual isA function to check for subclasses instead of doing a range comparison: The desired target class type is captured in the classof call. Then the || Parent::isA chain builds up a list of actual classes that this object is an instance of. If the captured type is found in this list, then the cast can proceed. You can see this in action in llvm/Support/Error.h (though the ErrorInfo class does not implement the classof method, because it wants to do casting differently.)

This is a bit more complex set up, but I think the overall result is worth it.

xiaobai updated this revision to Diff 203420.Jun 6 2019, 12:38 PM

Implement labath's suggestion

labath accepted this revision.Jun 7 2019, 9:30 AM

lgtm

source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
1128 ↗(On Diff #203420)

nit: the code (in this whole file) is clearly assuming the pointer is non-null, so you don't need the _or_null part. :)

This revision is now accepted and ready to land.Jun 7 2019, 9:30 AM
JDevlieghere accepted this revision.Jun 7 2019, 10:24 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2019, 11:42 AM