rjmccall added a comment. In https://reviews.llvm.org/D52674#1291284, @smeenai wrote:
> 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. Well, you could check for an `ObjCObjectPointerType` in the top-level routine. But yeah, probably the cleanest thing to do would to be push the state through the main dispatch. Don't push a single `bool` through, though; make a `TypeManglingOptions` struct to encapsulate it, in case we have a similar problem elsewhere. It should have a method for getting options that should be propagated down to component type manglings, and that method can drop the "for RTTI" bit; that's the part that `ObjCObjectPointerType` can handle differently. 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
