<div class="gmail_extra"><div class="gmail_quote">On Sat, Sep 1, 2012 at 8:13 PM, Joao Matos <span dir="ltr"><<a href="mailto:ripzonetriton@gmail.com" target="_blank" class="cremed">ripzonetriton@gmail.com</a>></span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: triton<br>
Date: Sat Sep  1 19:13:48 2012<br>
New Revision: 163078<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=163078&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=163078&view=rev</a><br>
Log:<br>
Added a diagnostic for mismatched MS inheritance attributes.</blockquote><div><br></div><div>You never posted an updated patch to John's review thread for this. He should have had a chance to sign off before you committed. =/ Please respect the commit-after-approval rules of the community.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Also fixed the incomplete type member pointer size calculation under the MS ABI.<br></blockquote><div><br></div>
<div>This should never have been in the same patch. Did it get reviewed anywhere?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
    cfe/trunk/lib/AST/MicrosoftCXXABI.cpp<br>
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
    cfe/trunk/lib/Sema/SemaType.cpp<br>
    cfe/trunk/test/SemaCXX/member-pointer-ms.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=163078&r1=163077&r2=163078&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=163078&r1=163077&r2=163078&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Sep  1 19:13:48 2012<br>
@@ -1672,6 +1672,9 @@<br>
<br>
 def err_attr_tlsmodel_arg : Error<"tls_model must be \"global-dynamic\", "<br>
   "\"local-dynamic\", \"initial-exec\" or \"local-exec\"">;<br>
+def warn_ms_inheritance_already_declared : Warning<<br>
+  "ignored since inheritance model was already declared as '"<br>
+  "%select{single|multiple|virtual}0'">;<br>
<br>
 def err_only_annotate_after_access_spec : Error<<br>
   "access specifier can only have annotation attributes">;<br>
<br>
Modified: cfe/trunk/lib/AST/MicrosoftCXXABI.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=163078&r1=163077&r2=163078&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=163078&r1=163077&r2=163078&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/AST/MicrosoftCXXABI.cpp (original)<br>
+++ cfe/trunk/lib/AST/MicrosoftCXXABI.cpp Sat Sep  1 19:13:48 2012<br>
@@ -55,6 +55,16 @@<br>
 unsigned MicrosoftCXXABI::getMemberPointerSize(const MemberPointerType *MPT) const {<br>
   QualType Pointee = MPT->getPointeeType();<br>
   CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl();<br>
+<br>
+  if (RD->getTypeForDecl()->isIncompleteType()) {<br>
+    if (RD->hasAttr<SingleInheritanceAttr>())<br>
+      return 1;<br>
+    else if (RD->hasAttr<MultipleInheritanceAttr>())<br>
+      return 2;<br>
+    else<br>
+      return 3;<br>
+  }<br>
+<br>
   if (RD->getNumVBases() > 0) {<br>
     if (Pointee->isFunctionType())<br>
       return 3;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=163078&r1=163077&r2=163078&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=163078&r1=163077&r2=163078&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Sep  1 19:13:48 2012<br>
@@ -4149,20 +4149,50 @@<br>
     S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << "uuid";<br>
 }<br>
<br>
+static bool hasOtherInheritanceAttr(Decl *D, AttributeList::Kind Kind,<br>
+    int& Existing) {<br>
+  if (Kind != AttributeList::AT_SingleInheritance &&<br>
+      D->hasAttr<SingleInheritanceAttr>()) {<br>
+    Existing = 0;<br>
+    return true;<br>
+  }<br>
+  else if (Kind != AttributeList::AT_MultipleInheritance &&<br>
+      D->hasAttr<MultipleInheritanceAttr>()) {<br>
+    Existing = 1;<br>
+    return true;<br>
+  }<br>
+  else if (Kind != AttributeList::AT_VirtualInheritance &&<br>
+      D->hasAttr<VirtualInheritanceAttr>()) {<br>
+    Existing = 2;<br>
+    return true;<br>
+  }<br>
+  return false;<br>
+}<br>
+<br>
 static void handleInheritanceAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
-  if (S.LangOpts.MicrosoftExt) {<br>
-    AttributeList::Kind Kind = Attr.getKind();<br>
-    if (Kind == AttributeList::AT_SingleInheritance)<br>
-      D->addAttr(<br>
-          ::new (S.Context) SingleInheritanceAttr(Attr.getRange(), S.Context));<br>
-    else if (Kind == AttributeList::AT_MultipleInheritance)<br>
-      D->addAttr(<br>
-          ::new (S.Context) MultipleInheritanceAttr(Attr.getRange(), S.Context));<br>
-    else if (Kind == AttributeList::AT_VirtualInheritance)<br>
-      D->addAttr(<br>
-          ::new (S.Context) VirtualInheritanceAttr(Attr.getRange(), S.Context));<br>
-  } else<br>
+  if (!S.LangOpts.MicrosoftExt) {<br>
     S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();<br>
+    return;<br>
+  }<br>
+<br>
+  AttributeList::Kind Kind = Attr.getKind();<br>
+<br>
+  int Existing;<br>
+  if (hasOtherInheritanceAttr(D->getCanonicalDecl(), Kind, Existing)) {<br>
+      S.Diag(Attr.getLoc(), diag::warn_ms_inheritance_already_declared) << Existing;<br>
+      return;<br>
+  }<br>
+<br>
+  if (Kind == AttributeList::AT_SingleInheritance) {<br>
+    D->addAttr(<br>
+        ::new (S.Context) SingleInheritanceAttr(Attr.getRange(), S.Context));<br>
+  } else if (Kind == AttributeList::AT_MultipleInheritance) {<br>
+    D->addAttr(<br>
+        ::new (S.Context) MultipleInheritanceAttr(Attr.getRange(), S.Context));<br>
+  } else if (Kind == AttributeList::AT_VirtualInheritance) {<br>
+    D->addAttr(<br>
+        ::new (S.Context) VirtualInheritanceAttr(Attr.getRange(), S.Context));<br>
+  }<br>
 }<br>
<br>
 static void handlePortabilityAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaType.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=163078&r1=163077&r2=163078&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=163078&r1=163077&r2=163078&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaType.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaType.cpp Sat Sep  1 19:13:48 2012<br>
@@ -1583,14 +1583,6 @@<br>
     return QualType();<br>
   }<br>
<br>
-  // In the Microsoft ABI, the class is allowed to be an incomplete<br>
-  // type. In such cases, the compiler makes a worst-case assumption.<br>
-  // We make no such assumption right now, so emit an error if the<br>
-  // class isn't a complete type.<br>
-  if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft &&<br>
-      RequireCompleteType(Loc, Class, diag::err_incomplete_type))<br>
-    return QualType();<br>
-<br>
   return Context.getMemberPointerType(T, Class.getTypePtr());<br>
 }<br>
<br>
<br>
Modified: cfe/trunk/test/SemaCXX/member-pointer-ms.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer-ms.cpp?rev=163078&r1=163077&r2=163078&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer-ms.cpp?rev=163078&r1=163077&r2=163078&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/test/SemaCXX/member-pointer-ms.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/member-pointer-ms.cpp Sat Sep  1 19:13:48 2012<br>
@@ -1,8 +1,4 @@<br>
-// RUN: %clang_cc1 -cxx-abi microsoft -fsyntax-only -verify %s<br>
-<br>
-// Test that we reject pointers to members of incomplete classes (for now)<br>
-struct A; //expected-note{{forward declaration of 'A'}}<br>
-int A::*pai1; //expected-error{{incomplete type 'A'}}<br>
+// RUN: %clang_cc1 -cxx-abi microsoft -fms-compatibility -fsyntax-only -verify %s<br>
<br>
 // Test that we don't allow reinterpret_casts from pointers of one size to<br>
 // pointers of a different size.<br>
@@ -12,3 +8,9 @@<br>
<br>
 void (A::*paf)();<br>
 void (C::*pcf)() = reinterpret_cast<void (C::*)()>(paf); //expected-error{{cannot reinterpret_cast from member pointer type}}<br>
+<br>
+class __single_inheritance D;<br>
+class __multiple_inheritance D; // expected-warning {{ignored since inheritance model was already declared as 'single'}}<br>
+<br>
+class __virtual_inheritance E;<br>
+class __virtual_inheritance E;  // no warning expected since same attribute<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>