smeenai added a comment. In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote:
> In https://reviews.llvm.org/D52674#1271814, @smeenai wrote: > > > @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? > > > Sorry for the delay. I think duplicating the dispatch logic for > `ObjCObjectPointerType` is probably the best approach; the pointee type will > always an `ObjCObjectType` of some sort, and there are only two kinds of > those. To be clear, we would need to duplicate (or factor out into a common function) some mangling logic as well, because e.g. adding the `E` In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1271814, @smeenai wrote: > > > @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? > > > Sorry for the delay. I think duplicating the dispatch logic for > `ObjCObjectPointerType` is probably the best approach; the pointee type will > always an `ObjCObjectType` of some sort, and there are only two kinds of > those. Sorry, I'm still not sure how this will work. Duplicating the dispatch logic for `ObjCObjectPointerType` ends up looking like https://reviews.llvm.org/P8114, which is fine. However, when we're actually mangling RTTI or RTTI names, we're still going through the main `mangleType(QualType, SourceRange, QualifierMangleMode)` overload, which still requires us to thread `ForRTTI` through that function, which still requires us to duplicate the switch in that function (to handle the `ForRTTI` case, since the other switch is generated via TypeNodes.def and not easily modifiable), which is the main ugliness in my opinion. Do you also want me to add special dispatching to `mangleCXXRTTI` and `mangleCXXRTTIName` to just call the `ObjCObjectPointerType` function directly when they're dealing with that type? That's certainly doable, but at that point just keeping some state around in the demangler starts to feel cleaner, at least to me. Repository: rC Clang https://reviews.llvm.org/D52674 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
