[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-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[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 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.
> >
> >
> > Hmm, but don't we want it to affect all components? For example, consider 
> > something like:
> >
> >   @interface I
> >   @end
> >  
> >   template  class C {};
> >  
> >   void f();
> >   void g() {
> > try {
> >   f();
> > } catch (C *) {
> > }
> >   }
> >
> >
> > I would say that we want the RTTI for `C *` to have the discriminator 
> > for `I`.
>
>
> Why?  IIUC, you're adding the discriminator to distinguish between two 
> different RTTI objects.  It's not like there are two different types.  You 
> should mangle `C` normally here.


You're right – I wasn't thinking this through correctly. Back to the drawing 
board.


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


[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 components of a 
> > type when we only mean to change it at the top level.
>
>
> Hmm, but don't we want it to affect all components? For example, consider 
> something like:
>
>   @interface I
>   @end
>  
>   template  class C {};
>  
>   void f();
>   void g() {
> try {
>   f();
> } catch (C *) {
> }
>   }
>
>
> I would say that we want the RTTI for `C *` to have the discriminator for 
> `I`.


Why?  IIUC, you're adding the discriminator to distinguish between two 
different RTTI objects.  It's not like there are two different types.  You 
should mangle `C` normally here.


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


[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.


Hmm, but don't we want it to affect all components? For example, consider 
something like:

  @interface I
  @end
  
  template  class C {};
  
  void f();
  void g() {
try {
  f();
} catch (C *) {
}
  }

I would say that we want the RTTI for `C *` to have the discriminator for 
`I`. It turns out my current patch doesn't actually do that; I guess there's a 
sub-mangler being constructed somewhere that's not inheriting the RTTI-ness. Or 
did you mean something else?


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


[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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[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 cleaner than having to 
thread through the RTTI state to every individual method. The ForRTTI_t enum is 
modeled after the ForDefinition_t enum used in CGM, but I'm happy to switch to 
a more general struct (as you'd mentioned before) if you prefer.


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


[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
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X86 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X64 %s
+
+// Ensure we have the __ObjC::class discriminator in the RTTI and the RTTI name.
+// X86-DAG: @"??_R0PAU?$Class@Uobjc_object@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [36 x i8] c".PAU?$Class@Uobjc_object@@@__ObjC@@\00" }, comdat
+// X64-DAG: @"??_R0PEAU?$Class@Uobjc_object@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [37 x i8] c".PEAU?$Class@Uobjc_object@@@__ObjC@@\00" }, comdat
+
+@class I;
+// X86-DAG: @"??_R0PAU?$Class@UI@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [26 x i8] c".PAU?$Class@UI@@@__ObjC@@\00" }, comdat
+// X64-DAG: @"??_R0PEAU?$Class@UI@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [27 x i8] c".PEAU?$Class@UI@@@__ObjC@@\00" }, comdat
+
+void f();
+void g() {
+  @try {
+f();
+  } @catch (I *) {
+  } @catch (id) {
+  }
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -252,6 +252,11 @@
 /// MicrosoftCXXNameMangler - Manage the mangling of a single name for the
 /// Microsoft Visual C++ ABI.
 class MicrosoftCXXNameMangler {
+public:
+  enum QualifierMangleMode { QMM_Drop, QMM_Mangle, QMM_Escape, QMM_Result };
+  enum ForRTTI_t : bool { NotForRTTI = false, ForRTTI = true };
+
+private:
   MicrosoftMangleContextImpl &Context;
   raw_ostream &Out;
 
@@ -276,25 +281,35 @@
   // this check into mangleQualifiers().
   const bool PointersAre64Bit;
 
-public:
-  enum QualifierMangleMode { QMM_Drop, QMM_Mangle, QMM_Escape, QMM_Result };
+  ForRTTI_t IsForRTTI;
 
+public:
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_)
   : Context(C), Out(Out_), Structor(nullptr), StructorType(-1),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
 
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
   const CXXConstructorDecl *D, CXXCtorType Type)
   : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
 
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
   const CXXDestructorDecl *D, CXXDtorType Type)
   : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
+
+  MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
+  ForRTTI_t IsForRTTI)
+  : Context(C), Out(Out_), Structor(nullptr), StructorType(-1),
+PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
+ 64),
+IsForRTTI(IsForRTTI) {}
 
   raw_ostream &getStream() const { return Out; }
 
@@ -339,6 +354,7 @@
   void
   mangleTemplateInstantiationName(const TemplateDecl *TD,
   const TemplateArgumentList &TemplateArgs);
+  void mangleObjCClassName(StringRef Name);
   void mangleObjCMethodName(const ObjCMethodDecl *MD);
 
   void mangleArgumentType(QualType T, SourceRange Range);
@@ -1277,6 +1293,27 @@
   }
 }
 
+void MicrosoftCXXNameMangler::mangleObjCClassName(StringRef Name) {
+  // Obj-C classes are normally mangled as C++ structs with the same name, but
+  // we want to be able to distinguish a C++ struct X from an Obj-C class X for
+  // the purposes of exception handling, so we add a discriminator when mangling
+  // for RTTI.
+  if (!IsForRTTI) {
+mangleArtificalTagType(TTK_Struct, Name);
+return;
+  }
+
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+
+  Stream << "?$";
+  Extra.mangleSourceName("Class");
+  Ex

[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-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[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:
> > >
> > > > @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.


All right, the struct is a good idea. It would require adding the 
`TypeManglingOptions` parameter to all the `mangleType` overloads (unless we 
want to stick with the second switch in the main `mangleType` dispatch 
function, but that seems super ugly, especially if we're doing a more general 
solution), so I wanted to confirm @rnk is okay with adding a parameter to all 
those overloads as well before proceeding.


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


[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://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


[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-bin/mailman/listinfo/cfe-commits


[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, 
> > 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

[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, 
> `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.


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


[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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[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 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[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 ultimate 
> > mangling function:
> >
> >   MicrosoftMangleContextImpl::mangleCXXRTTI(QualType, raw_ostream &)
> >   MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
> > QualifierMangleMode)
> >   MicrosoftCXXNameMangler::mangleType(const ObjCObjectPointerType *, 
> > Qualifiers, SourceRange)
> >   MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
> > QualifierMangleMode)
> >   MicrosoftCXXNameMangler::mangleType(const ObjCObjectType *, Qualifiers, 
> > SourceRange)
> >
> >
> > (the last one will be `ObjCInterfaceType` instead of `ObjCObjectType` if 
> > catching anything other than `id`)
> >
> > Threading a `ForRTTI` flag or similar all the way to the final call seems 
> > pretty tricky. I can add an optional paramater to `mangleType(QualType, 
> > SourceRange, QualifierMangleMode)`, but that function uses a generated 
> > switch case 
> > 
> >  to call the specific `mangleType` functions, and I don't know how to 
> > special-case certain types in that switch case (to pass the extra parameter 
> > along) without doing something super ugly. Adding the extra parameter to 
> > every single `mangleType` overload seems highly non-ideal, which is why I 
> > was thinking of maintaining some internal state instead.
>
>
> Well, that's why I was talking about how the pointee type of an 
> `ObjCObjectPointerType` is always an `ObjCObjectType` (of which 
> `ObjCInterfaceType` is a subclass) — the implication being that you don't 
> actually have to do the `switch` to dispatch to one of those two cases.


The intermediate `mangleType(QualType, SourceRange, QualifierMangleMode)` calls 
have a bunch of logic 

 in them; it's not just a direct dispatch to the actual mangling function, so I 
don't think we can skip over it. I think it's hard to discuss this in the 
abstract though (and it's also entirely possible I'm missing your point 
completely), so I'll just actually try out the parameter approach and put up 
the resulting patch and we can see how it turns out.


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


[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::mangleCXXRTTI(QualType, raw_ostream &)
>   MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
> QualifierMangleMode)
>   MicrosoftCXXNameMangler::mangleType(const ObjCObjectPointerType *, 
> Qualifiers, SourceRange)
>   MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
> QualifierMangleMode)
>   MicrosoftCXXNameMangler::mangleType(const ObjCObjectType *, Qualifiers, 
> SourceRange)
>
>
> (the last one will be `ObjCInterfaceType` instead of `ObjCObjectType` if 
> catching anything other than `id`)
>
> Threading a `ForRTTI` flag or similar all the way to the final call seems 
> pretty tricky. I can add an optional paramater to `mangleType(QualType, 
> SourceRange, QualifierMangleMode)`, but that function uses a generated switch 
> case 
> 
>  to call the specific `mangleType` functions, and I don't know how to 
> special-case certain types in that switch case (to pass the extra parameter 
> along) without doing something super ugly. Adding the extra parameter to 
> every single `mangleType` overload seems highly non-ideal, which is why I was 
> thinking of maintaining some internal state instead.


Well, that's why I was talking about how the pointee type of an 
`ObjCObjectPointerType` is always an `ObjCObjectType` (of which 
`ObjCInterfaceType` is a subclass) — the implication being that you don't 
actually have to do the `switch` to dispatch to one of those two cases.


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


[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 &)
  MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
QualifierMangleMode)
  MicrosoftCXXNameMangler::mangleType(const ObjCObjectPointerType *, 
Qualifiers, SourceRange)
  MicrosoftCXXNameMangler::mangleType(QualType, SourceRange, 
QualifierMangleMode)
  MicrosoftCXXNameMangler::mangleType(const ObjCObjectType *, Qualifiers, 
SourceRange)

(the last one will be `ObjCInterfaceType` instead of `ObjCObjectType` if 
catching anything other than `id`)

Threading a `ForRTTI` flag or similar all the way to the final call seems 
pretty tricky. I can add an optional paramater to `mangleType(QualType, 
SourceRange, QualifierMangleMode)`, but that function uses a generated switch 
case 

 to call the specific `mangleType` functions, and I don't know how to 
special-case certain types in that switch case (to pass the extra parameter 
along) without doing something super ugly. Adding the extra parameter to every 
single `mangleType` overload seems highly non-ideal, which is why I was 
thinking of maintaining some internal state instead.


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


[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, 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 crossing its 
> > > fingers.
> >
> >
> > That's a fair concern.
> >
> > I believe most of the Obj-C extensions have been handled by @compnerd, and 
> > he's been following a pretty consistent scheme using the `__Objc` 
> > namespace, e.g. `void f(id) {}` is mangled as `void __cdecl f(struct 
> > objc_object > *)`. I could certainly try 
> > to implement something similar here, except as I mentioned, I'm pretty sure 
> > it would require maintaining some state in the demangler for indicating 
> > whether we were mangling for RTTI.
>
>
> State wouldn't really be the right solution anyway, although it might work 
> because of the structural restrictions on Objective-C types.  If you wanted 
> to keep that same rule, I think you could probably just pass a 
> (default=`false`) flag down to the `ObjCObjectPointerType`, `ObjCObjectType`, 
> and `ObjCInterfaceType` cases.  (The pointee type of the former will always 
> be one of the latter two.)


Ah, there are fewer hops between the `mangleType` call and its ultimate 
destination than I was expecting, so a default parameter would work. I'll 
change this accordingly, thanks.


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


[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.  Currently it 
> > feels like every patch to add a Clang extension to the Microsoft mangling 
> > ends up inventing its own rules and crossing its fingers.
>
>
> That's a fair concern.
>
> I believe most of the Obj-C extensions have been handled by @compnerd, and 
> he's been following a pretty consistent scheme using the `__Objc` namespace, 
> e.g. `void f(id) {}` is mangled as `void __cdecl f(struct 
> objc_object > *)`. I could certainly try to 
> implement something similar here, except as I mentioned, I'm pretty sure it 
> would require maintaining some state in the demangler for indicating whether 
> we were mangling for RTTI.


State wouldn't really be the right solution anyway, although it might work 
because of the structural restrictions on Objective-C types.  If you wanted to 
keep that same rule, I think you could probably just pass a (default=`false`) 
flag down to the `ObjCObjectPointerType`, `ObjCObjectType`, and 
`ObjCInterfaceType` cases.  (The pointee type of the former will always be one 
of the latter two.)


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


[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 the Microsoft mangling 
> ends up inventing its own rules and crossing its fingers.


That's a fair concern.

I believe most of the Obj-C extensions have been handled by @compnerd, and he's 
been following a pretty consistent scheme using the `__Objc` namespace, e.g. 
`void f(id) {}` is mangled as `void __cdecl f(struct objc_object > *)`. I could certainly try to implement something 
similar here, except as I mentioned, I'm pretty sure it would require 
maintaining some state in the demangler for indicating whether we were mangling 
for RTTI.


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


[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 crossing its fingers.


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


[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
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -fobjc-runtime=gnustep-2.0 
-fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X86 
%s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fobjc-runtime=gnustep-2.0 
-fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X64 
%s
+
+// Ensure we have the .objc discriminator in the RTTI and the RTTI name.
+// X86-DAG: @"??_R0objc.PAUobjc_object@@@8" = linkonce_odr global %{{[^ ]+}} { 
i8** @"??_7type_info@@6B@", i8* null, [23 x i8] c".objc.PAUobjc_object@@\00" }, 
comdat
+// X64-DAG: @"??_R0objc.PEAUobjc_object@@@8" = linkonce_odr global %{{[^ ]+}} 
{ i8** @"??_7type_info@@6B@", i8* null, [24 x i8] c".objc.PEAUobjc_object@@\00" 
}, comdat
+
+@class I;
+// X86-DAG: @"??_R0objc.PAUI@@@8" = linkonce_odr global %{{[^ ]+}} { i8** 
@"??_7type_info@@6B@", i8* null, [13 x i8] c".objc.PAUI@@\00" }, comdat
+// X64-DAG: @"??_R0objc.PEAUI@@@8" = linkonce_odr global %{{[^ ]+}} { i8** 
@"??_7type_info@@6B@", i8* null, [14 x i8] c".objc.PEAUI@@\00" }, comdat
+
+void f();
+void g() {
+  @try {
+f();
+  } @catch (I *) {
+  } @catch (id) {
+  }
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -3001,14 +3001,22 @@
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO);
   Mangler.getStream() << "??_R0";
+  // Obj-C classes are mangled as C++ structs with the same name, but we want 
to
+  // be able to distinguish a C++ struct X from an Obj-C class X for the
+  // purposes of exception handling, so we add a discriminator.
+  if (T->isObjCObjectPointerType())
+Mangler.getStream() << "objc.";
   Mangler.mangleType(T, SourceRange(), MicrosoftCXXNameMangler::QMM_Result);
   Mangler.getStream() << "@8";
 }
 
 void MicrosoftMangleContextImpl::mangleCXXRTTIName(QualType T,
raw_ostream &Out) {
   MicrosoftCXXNameMangler Mangler(*this, Out);
   Mangler.getStream() << '.';
+  // See the comment in MicrosoftMangleContextImpl::mangleCXXRTTI.
+  if (T->isObjCObjectPointerType())
+Mangler.getStream() << "objc.";
   Mangler.mangleType(T, SourceRange(), MicrosoftCXXNameMangler::QMM_Result);
 }
 


Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X86 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X64 %s
+
+// Ensure we have the .objc discriminator in the RTTI and the RTTI name.
+// X86-DAG: @"??_R0objc.PAUobjc_object@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [23 x i8] c".objc.PAUobjc_object@@\00" }, comdat
+// X64-DAG: @"??_R0objc.PEAUobjc_object@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [24 x i8] c".objc.PEAUobjc_object@@\00" }, comdat
+
+@class I;
+// X86-DAG: @"??_R0objc.PAUI@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [13 x i8] c".objc.PAUI@@\00" }, comdat
+// X64-DAG: @"??_R0objc.PEAUI@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [14 x i8] c".objc.PEAUI@@\00" }, comdat
+
+void f();
+void g() {
+  @try {
+f();
+  } @catch (I *) {
+  } @catch (id) {
+  }
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -3001,14 +3001,22 @@
   msvc_hashing_ostream MHO(Out);
   MicrosoftCXXNameMangler Mangler(*this, MHO);
   Mangler.getStream() << "??_R0";
+  // Obj-C classes are mangled as C++ structs with the same name, but we want to
+  // be able to distinguish a C++ struct X from an Obj-C class X for the
+  // purposes of exception handling, so we add a discriminator.
+  if (T->isObjCObjectPointerType())
+Mangler.getStream() << "objc.";
   Mangler.mangleType(T, SourceRange(), MicrosoftCXXNameMangler::QMM_Result);
   Mangler.getStream() << "@8";
 }
 
 void MicrosoftMangleContextImpl::mangleCXXRTTIName(QualType T,
raw_ostream &Out) {
   MicrosoftCXXNameMangler Mangler(*this, Out);
   Mangler.

[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 for the purposes of exception handling, so we need to add a
discriminator to the RTTI and the RTTI name.

The chosen discriminator (`.objc`) is fairly arbitrary, and I'm open to
other suggestions. It's also perhaps not ideal to have the discriminator
as a prefix, since then the RTTI won't demangle (RTTI names never
demangle anyway, so that's less of a concern). Having it infix would
make for a cleaner mangling, but it would also require keeping some
state around in the mangler to indicate when we're mangling for RTTI,
which seems like a much uglier implementation and not worth it.


Repository:
  rC Clang

https://reviews.llvm.org/D52674

Files:
  lib/AST/MicrosoftMangle.cpp
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
+// CHECK: @"?kI@@3PAUI@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAXABUI@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?i@@YAPAUI@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?j@@YAPBUI@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?k@@YAAAUI@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
+// CHECK-LABEL: "?l@@YAABUI@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
+// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
 
   void m(I &) {}
-  // CHECK-LABE