<div dir="ltr">On Fri, Mar 22, 2013 at 4:44 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mar 22, 2013, at 11:41 AM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>

>    - Update the test case to pass under x86_64.<br>
><br>
> Hi rjmccall,<br>
><br>
> <a href="http://llvm-reviews.chandlerc.com/D568" target="_blank">http://llvm-reviews.chandlerc.com/D568</a><br>
<br>
</div>+    // FIXME: Handling x86_64 will require changing this method to return sizes<br>
+    // in bytes instead of pointers.  Some member pointer sizes are not<br>
+    // multiples of a pointer size.<br>
<br>
So, change the method to return sizes in bytes instead of pointers instead<br>
of reporting a diagnostic in a core AST routine.<br></blockquote><div><br></div><div style>OK, I'll change the method.  I assumed there'd be more than one caller.  :)</div><div><br></div><div style>What should I do for non-x86 architectures then?  I could assert, since presumably we'll have errorred out much earlier if someone asks the frontend for mips code.</div>
<div style><br></div><div style>There's prior art for issuing diagnostics in AST from RecordLayoutBuilder, but maybe that's different.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+  attr::Kind Inheritance;<br>
<br>
+      // This last case seems like an error, but MSVC allocates one more slot<br>
+      // than is used for virtual inheritance.  We follow suit for<br>
+      // compatibility.<br>
<br>
My understanding is that it's required for vtordisps or something.<br>
<br>
+      // FIXME: Issue a warning.<br>
<br>
Well, you wouldn't issue a warning in the AST library, so the FIXME doesn't<br>
go here.<br>
<br>
+    if (Attr *IA = RD->getAttr<SingleInheritanceAttr>())<br>
+      Inheritance = IA->getKind();<br>
+    else if (Attr *IA = RD->getAttr<MultipleInheritanceAttr>())<br>
+      Inheritance = IA->getKind();<br>
+    else if (Attr *IA = RD->getAttr<VirtualInheritanceAttr>())<br>
+      Inheritance = IA->getKind();<br>
<br>
You should really make a common base class for these attributes.<br>
I believe this is just as simple as adding:<br>
  class MSInheritanceAttr : InheritableAttr;<br>
to Attr.td, making the attribute defs inherit from that, and then defining that<br>
class in Attr.h.<br></blockquote><div style><br></div><div style>I can do that.  It'll be in the next patch, but I'll commit it separately.</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+// getNumBases() seems to only give us the number of direct bases, and not the<br>
+// total.  This function tells us if we inherit from anybody that uses MI.<br>
+static bool recordHasMultipleInheritance(const CXXRecordDecl *RD) {<br>
+  while (RD->getNumBases() > 0) {<br>
+    if (RD->getNumBases() > 1)<br>
+      return true;<br>
+    assert(RD->getNumBases() == 1);<br>
+    RD = RD->bases_begin()->getType()->getAsCXXRecordDecl();<br>
+  }<br>
+  return false;<br>
+}<br>
<br>
Hmm.  I bet that non-primary base class forces the "multiple inheritance" case:<br>
  struct A { int x; void bar(); };<br>
  struct B : A { virtual void foo(); };<br>
What's sizeof(void (B::*)())?<br></blockquote><div><br></div><div style>Nice catch, 2 * sizeof(void*), so it's the "multiple" case.  Added a test case.</div><div style><br></div><div style>What do I need to look for to identify this case?  A class that is polymorphic, but whose base is not?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+  switch (Inheritance) {<br>
+  case attr::SingleInheritance:<br>
+    return 1;<br>
+  case attr::MultipleInheritance:<br>
+    return 1 + int(Pointee->isFunctionType());<br>
+  case attr::VirtualInheritance:<br>
+    return 2 + int(Pointee->isFunctionType());<br>
<br>
Please try to avoid doing the somewhat-expensive query about multiple<br>
inheritance if this is a non-function member pointer.<br></blockquote><div><br></div><div style>OK.</div></div></div></div>