[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

Shoaib Meenai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 22 18:20:30 PDT 2018


smeenai added a comment.

@rjmccall I prototyped the ForRTTI parameter approach in https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, but it demonstrates the problems I saw with the added parameter. Namely, `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of additional logic which is needed for correctness, so we need to thread the parameter through the entire chain, and the only way I could think of doing that without adding the parameter to every single `mangleType` overload was to have an additional switch to dispatch to the overloads with the added ForRTTI parameter, which is pretty ugly.

As I see it, there are a few ways to proceed here:

- The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I think it's pretty ugly, but you may have suggestions on how to do it better.
- In the `mangleType` overload for `ObjCObjectPointerType`, skipping the generic `mangleType` dispatch and going directly to either the `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the ugliness in the generic `mangleType`, but it also requires us to duplicate some logic from it in the `ObjCObjectPointerType` overload, which doesn't seem great either.
- Maintaining `ForRTTI` state in the mangler instead of threading a parameter through (I'm generally not a fan of state like that, but it might be cleaner than the threading in this case?)
- Just sticking with what I'm doing in this patch.

What do you think?


Repository:
  rC Clang

https://reviews.llvm.org/D52674





More information about the cfe-commits mailing list