Using llvm-style rtti gives us stronger guarantees around casting
LanguageRuntimes.
As discussed in D62755
Details
Details
Diff Detail
Diff Detail
- Build Status
Buildable 32963 Build 32962: arc lint + arc unit
Event Timeline
Comment Actions
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.
Comment Actions
lgtm
| source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp | ||
|---|---|---|
| 1126–1128 | nit: the code (in this whole file) is clearly assuming the pointer is non-null, so you don't need the _or_null part. :) | |
nit: the code (in this whole file) is clearly assuming the pointer is non-null, so you don't need the _or_null part. :)