[PATCH] MS RTTI Generation

[Reid Kleckner]

lgtm

Comments are only mechanical issues.

================
Comment at: lib/CodeGen/CodeGenModule.h:713-718
@@ -712,2 +712,8 @@
 
+  /// \brief Gets or a creats a Microsoft TypeDescriptor.
+  llvm::Constant *getMSTypeDescriptor(QualType Ty);
+  /// \brief Gets or a creats a Microsoft CompleteObjectLocator.
+  llvm::GlobalVariable *getMSCompleteObjectLocator(const CXXRecordDecl *RD,
+                                                   const VPtrInfo *Info);
+
   /// Gets the address of a block which requires no captures.
----------------
Can you move these near GetAddrOfRTTIDescriptor?  This is an ABI-specific interface, but without exposing MicrosoftCXXABI or merging MicrosoftRTTI.cpp with MicrosoftCXXABI.cpp, we can't reach it any other way.

================
Comment at: lib/CodeGen/CodeGenModule.h:954
@@ -947,1 +953,3 @@
 
+  /// \brief Returns the approrpiate linkage for the TypeInfo struct for a type.
+  llvm::GlobalVariable::LinkageTypes getTypeInfoLinkage(QualType Ty);
----------------
s/approrpiate/appropriate/

================
Comment at: lib/CodeGen/MicrosoftRTTI.cpp:24
@@ +23,3 @@
+
+namespace {
+
----------------
There's a guideline to "make anonymous namespaces as small as possible":
http://llvm.org/docs/CodingStandards.html#anonymous-namespaces

Most of the free functions should be static (I think they were before?), and just the class definition (not the method implementations) should be anonymous.

================
Comment at: lib/CodeGen/MicrosoftRTTI.cpp:46
@@ +45,3 @@
+//   every path in the hierarchy, in pre-order depth first order.  Note, we do
+//   not declare a specific llvm type for BaseClassArray, it's mearly an array
+//   of BaseClassDescriptor pointers.
----------------
s/mearly/merely/

================
Comment at: lib/CodeGen/MicrosoftRTTI.cpp:52
@@ +51,3 @@
+//   BaseClassArray is.  It contains information about a class within a
+//   hierarchy such as if this base is ambiguous and what it's offset in the
+//   vbtable.  The names of the BaseClassDescriptors have all of their fields
----------------
s/what it's/what is it's/

================
Comment at: lib/CodeGen/MicrosoftRTTI.cpp:60
@@ +59,3 @@
+llvm::StructType *getTypeDescriptorType(CodeGenModule &CGM,
+                                               StringRef Str) {
+  llvm::SmallString<32> TDTypeName("MSRTTITypeDescriptor");
----------------
I'd rename Str to something informative, like maybe MangledTypeName?

nit: reflow

================
Comment at: lib/CodeGen/MicrosoftRTTI.cpp:76-83
@@ +75,10 @@
+    return Type;
+  SmallVector<llvm::Type *, 16> FieldTypes;
+  FieldTypes.push_back(CGM.Int8PtrTy);
+  FieldTypes.push_back(CGM.IntTy);
+  FieldTypes.push_back(CGM.IntTy);
+  FieldTypes.push_back(CGM.IntTy);
+  FieldTypes.push_back(CGM.IntTy);
+  FieldTypes.push_back(CGM.IntTy);
+  FieldTypes.push_back(getClassHierarchyDescriptorType(CGM)->getPointerTo());
+  return llvm::StructType::create(CGM.getLLVMContext(), FieldTypes, Name);
----------------
Any reason not to write:

  llvm::Type *FieldTypes[] = {
    CGM.Int8PtrTy,
    CGM.IntTy,
    CGM.IntTy,
    CGM.IntTy,
    CGM.IntTy,
    CGM.IntTy,
    getClassHierarchyDescriptorType(CGM)->getPointerTo()
  };

?

This pattern appears in lots of other places too.

http://reviews.llvm.org/D3833