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

2018-11-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Well, you could go further down the route of what we do for "structors", and store the top-level decl being mangled in the mangler. Would that solve the problem? Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-

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

2018-11-15 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. In https://reviews.llvm.org/D52674#1298115, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1297893, @smeenai wrote: > > > In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote: > > > > > I'm not worried about the mang

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

2018-11-14 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D52674#1297893, @smeenai wrote: > In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote: > > > I'm not worried about the mangler being re-used for multiple declarations, > > I'm worried about a global flag changing how we mangle all com

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

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote: > I'm not worried about the mangler being re-used for multiple declarations, > I'm worried about a global flag changing how we mangle all components of a > type when we only mean to change it at the top level.

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

2018-11-13 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I'm not worried about the mangler being re-used for multiple declarations, I'm worried about a global flag changing how we mangle all components of a type when we only mean to change it at the top level. Repository: rC Clang https://reviews.llvm.org/D52674 _

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

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. @rnk pointed out on IRC that the MicrosoftCXXNameMangler is actually specifically designed to manage the mangling of only a single name, in which case adding state to it for handling RTTI seems like a natural approach. @rjmccall, what do you think? I think this is much

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

2018-11-13 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 173965. smeenai added a comment. Stateful approach Repository: rC Clang https://reviews.llvm.org/D52674 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm

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

2018-11-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Threading a new options argument through mangleType that includes QualifierMangleMode as well as these obj-c options seems reasonable. Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-commits mailing list cfe-com

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

2018-11-09 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1293180, @rjmccall wrote: > 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: > > > > > >

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

2018-11-09 Thread John McCall via Phabricator via cfe-commits
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://

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

2018-11-07 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Sorry, had a leftover draft which I forgot to clean up. Edited in Phabricator now. Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

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

2018-11-07 Thread Shoaib Meenai via Phabricator via cfe-commits
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, > > bu

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

2018-10-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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,

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

2018-10-30 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Ping @rjmccall. Let me know if the approach in https://reviews.llvm.org/D53546 is what you'd been envisioning, or if I'm just doing something completely brain-dead :) Repository: rC Clang https://reviews.llvm.org/D52674 ___

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

2018-10-22 Thread Shoaib Meenai via Phabricator via cfe-commits
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 bun

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

2018-10-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1253408, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1253401, @smeenai wrote: > > > Actually, I take that back ... I just misread the stack trace. > > > > There are a bunch of hops between the `mangleCXXRTTI` call and the ultim

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

2018-10-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D52674#1253401, @smeenai wrote: > Actually, I take that back ... I just misread the stack trace. > > There are a bunch of hops between the `mangleCXXRTTI` call and the ultimate > mangling function: > > MicrosoftMangleContextImpl::mangleCXXR

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

2018-10-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai requested review of this revision. smeenai added a comment. Actually, I take that back ... I just misread the stack trace. There are a bunch of hops between the `mangleCXXRTTI` call and the ultimate mangling function: MicrosoftMangleContextImpl::mangleCXXRTTI(QualType, raw_ostream &)

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

2018-10-02 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. In https://reviews.llvm.org/D52674#1251931, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1251439, @smeenai wrote: > > > In https://reviews.llvm.org/D52674#1251419, @rjmccall wrote: > > > > > Conceptually this seems fine,

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

2018-10-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D52674#1251439, @smeenai wrote: > In https://reviews.llvm.org/D52674#1251419, @rjmccall wrote: > > > Conceptually this seems fine, but I think it would be good to stop and make > > sure we're using a consistent style when mangling extensions.

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

2018-10-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D52674#1251419, @rjmccall wrote: > Conceptually this seems fine, but I think it would be good to stop and make > sure we're using a consistent style when mangling extensions. Currently it > feels like every patch to add a Clang extension to

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

2018-10-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Conceptually this seems fine, but I think it would be good to stop and make sure we're using a consistent style when mangling extensions. Currently it feels like every patch to add a Clang extension to the Microsoft mangling ends up inventing its own rules and crossin

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

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 167555. smeenai added a comment. arc fail Repository: rC Clang https://reviews.llvm.org/D52674 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm ===

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

2018-09-28 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: compnerd, DHowett-MSFT, rjmccall, rnk, theraven. Herald added a subscriber: erik.pilkington. Obj-C classes are mangled as C++ structs with the same name (in both the Itanium and the Microsoft ABIs), but we want to be able to distinguish them