[PATCH] [ms-cxxabi] Correctly compute the size of member pointers

[John McCall]

On Mar 22, 2013, at 4:50 PM, Reid Kleckner <rnk at google.com> wrote:
>    - Implement John's comments.

-    std::pair<uint64_t, unsigned> PtrDiffInfo =
-      getTypeInfo(getPointerDiffType());
-    Width = PtrDiffInfo.first * ABI->getMemberPointerSize(MPT);
-    Align = PtrDiffInfo.second;
+    Width = ABI->getMemberPointerWidth(MPT);
+    Align = ABI->getMemberPointerAlign(MPT);

Please have one function that returns both of these.  No reason to
do two virtual calls when we can do one.

This will also allow you to get the alignment right more easily on 64-bit.

+static bool usesMultipleInheritanceModel(const MemberPointerType *MPT,
+                                         const CXXRecordDecl *RD) {
+  // For data pointers, the representation is the same for single and multiple
+  // inheritance classes.  We return false to use the single inheritance model.
+  if (MPT->isMemberDataPointerType())
+    return false;
+

This isn't quite what I meant.  The top-level routine already decides whether
the member type is a function or not.  Just don't call this function if it is.

That is, change this:
+    else if (usesMultipleInheritanceModel(MPT, RD))
to
 +    else if (isFunc && usesMultipleInheritanceModel(MPT, RD))

+  if (RD->getTypeForDecl()->isIncompleteType()) {
+    if (Attr *IA = RD->getAttr<SingleInheritanceAttr>())
+      Inheritance = IA->getKind();
+    else if (Attr *IA = RD->getAttr<MultipleInheritanceAttr>())
+      Inheritance = IA->getKind();
+    else if (Attr *IA = RD->getAttr<VirtualInheritanceAttr>())
+      Inheritance = IA->getKind();
+    else
+      Inheritance = attr::UnspecifiedInheritance;
+  } else {

Check for the attribute first, and check for it with:
  if (Attr *IA = RD->getAttr<MSInheritanceAttr>()) {
  }
That way you can e.g. implicitly add your UnspecifiedInheritanceAttr to a
class decl and it'll just work.  Also it means you don't have to enumerate your
inheritance attribute kinds.

+  // If template instantiation fails or the type is just incomplete, the ABI has
+  // a poorly understood fallback mechanism in place, so we continue without any
+  // diagnostics.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+    RequireCompleteType(Loc, Class, 0);

This would be an excellent opportunity to implicitly add your
UnspecifiedInheritanceAttr to all the existing declarations. :)

The changes to ClangAttrEmitter look fine.

John.