You are right, I'm sorry for having commited this. I had already addressed John's concern, but used the wrong patch. I attached it for review.
On Tue, Sep 4, 2012 at 5:13 PM, Chandler Carruth <[email protected]>wrote: > On Sat, Sep 1, 2012 at 8:13 PM, Joao Matos <[email protected]>wrote: > >> Author: triton >> Date: Sat Sep 1 19:13:48 2012 >> New Revision: 163078 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=163078&view=rev >> Log: >> Added a diagnostic for mismatched MS inheritance attributes. > > > 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. > > >> Also fixed the incomplete type member pointer size calculation under the >> MS ABI. >> > > This should never have been in the same patch. Did it get reviewed > anywhere? > > >> >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/AST/MicrosoftCXXABI.cpp >> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> cfe/trunk/lib/Sema/SemaType.cpp >> cfe/trunk/test/SemaCXX/member-pointer-ms.cpp >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=163078&r1=163077&r2=163078&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Sep 1 >> 19:13:48 2012 >> @@ -1672,6 +1672,9 @@ >> >> def err_attr_tlsmodel_arg : Error<"tls_model must be \"global-dynamic\", >> " >> "\"local-dynamic\", \"initial-exec\" or \"local-exec\"">; >> +def warn_ms_inheritance_already_declared : Warning< >> + "ignored since inheritance model was already declared as '" >> + "%select{single|multiple|virtual}0'">; >> >> def err_only_annotate_after_access_spec : Error< >> "access specifier can only have annotation attributes">; >> >> Modified: cfe/trunk/lib/AST/MicrosoftCXXABI.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=163078&r1=163077&r2=163078&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/MicrosoftCXXABI.cpp (original) >> +++ cfe/trunk/lib/AST/MicrosoftCXXABI.cpp Sat Sep 1 19:13:48 2012 >> @@ -55,6 +55,16 @@ >> unsigned MicrosoftCXXABI::getMemberPointerSize(const MemberPointerType >> *MPT) const { >> QualType Pointee = MPT->getPointeeType(); >> CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl(); >> + >> + if (RD->getTypeForDecl()->isIncompleteType()) { >> + if (RD->hasAttr<SingleInheritanceAttr>()) >> + return 1; >> + else if (RD->hasAttr<MultipleInheritanceAttr>()) >> + return 2; >> + else >> + return 3; >> + } >> + >> if (RD->getNumVBases() > 0) { >> if (Pointee->isFunctionType()) >> return 3; >> >> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=163078&r1=163077&r2=163078&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Sep 1 19:13:48 2012 >> @@ -4149,20 +4149,50 @@ >> S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << "uuid"; >> } >> >> +static bool hasOtherInheritanceAttr(Decl *D, AttributeList::Kind Kind, >> + int& Existing) { >> + if (Kind != AttributeList::AT_SingleInheritance && >> + D->hasAttr<SingleInheritanceAttr>()) { >> + Existing = 0; >> + return true; >> + } >> + else if (Kind != AttributeList::AT_MultipleInheritance && >> + D->hasAttr<MultipleInheritanceAttr>()) { >> + Existing = 1; >> + return true; >> + } >> + else if (Kind != AttributeList::AT_VirtualInheritance && >> + D->hasAttr<VirtualInheritanceAttr>()) { >> + Existing = 2; >> + return true; >> + } >> + return false; >> +} >> + >> static void handleInheritanceAttr(Sema &S, Decl *D, const AttributeList >> &Attr) { >> - if (S.LangOpts.MicrosoftExt) { >> - AttributeList::Kind Kind = Attr.getKind(); >> - if (Kind == AttributeList::AT_SingleInheritance) >> - D->addAttr( >> - ::new (S.Context) SingleInheritanceAttr(Attr.getRange(), >> S.Context)); >> - else if (Kind == AttributeList::AT_MultipleInheritance) >> - D->addAttr( >> - ::new (S.Context) MultipleInheritanceAttr(Attr.getRange(), >> S.Context)); >> - else if (Kind == AttributeList::AT_VirtualInheritance) >> - D->addAttr( >> - ::new (S.Context) VirtualInheritanceAttr(Attr.getRange(), >> S.Context)); >> - } else >> + if (!S.LangOpts.MicrosoftExt) { >> S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << >> Attr.getName(); >> + return; >> + } >> + >> + AttributeList::Kind Kind = Attr.getKind(); >> + >> + int Existing; >> + if (hasOtherInheritanceAttr(D->getCanonicalDecl(), Kind, Existing)) { >> + S.Diag(Attr.getLoc(), diag::warn_ms_inheritance_already_declared) >> << Existing; >> + return; >> + } >> + >> + if (Kind == AttributeList::AT_SingleInheritance) { >> + D->addAttr( >> + ::new (S.Context) SingleInheritanceAttr(Attr.getRange(), >> S.Context)); >> + } else if (Kind == AttributeList::AT_MultipleInheritance) { >> + D->addAttr( >> + ::new (S.Context) MultipleInheritanceAttr(Attr.getRange(), >> S.Context)); >> + } else if (Kind == AttributeList::AT_VirtualInheritance) { >> + D->addAttr( >> + ::new (S.Context) VirtualInheritanceAttr(Attr.getRange(), >> S.Context)); >> + } >> } >> >> static void handlePortabilityAttr(Sema &S, Decl *D, const AttributeList >> &Attr) { >> >> Modified: cfe/trunk/lib/Sema/SemaType.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=163078&r1=163077&r2=163078&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaType.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaType.cpp Sat Sep 1 19:13:48 2012 >> @@ -1583,14 +1583,6 @@ >> return QualType(); >> } >> >> - // In the Microsoft ABI, the class is allowed to be an incomplete >> - // type. In such cases, the compiler makes a worst-case assumption. >> - // We make no such assumption right now, so emit an error if the >> - // class isn't a complete type. >> - if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft && >> - RequireCompleteType(Loc, Class, diag::err_incomplete_type)) >> - return QualType(); >> - >> return Context.getMemberPointerType(T, Class.getTypePtr()); >> } >> >> >> Modified: cfe/trunk/test/SemaCXX/member-pointer-ms.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-pointer-ms.cpp?rev=163078&r1=163077&r2=163078&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/member-pointer-ms.cpp (original) >> +++ cfe/trunk/test/SemaCXX/member-pointer-ms.cpp Sat Sep 1 19:13:48 2012 >> @@ -1,8 +1,4 @@ >> -// RUN: %clang_cc1 -cxx-abi microsoft -fsyntax-only -verify %s >> - >> -// Test that we reject pointers to members of incomplete classes (for >> now) >> -struct A; //expected-note{{forward declaration of 'A'}} >> -int A::*pai1; //expected-error{{incomplete type 'A'}} >> +// RUN: %clang_cc1 -cxx-abi microsoft -fms-compatibility -fsyntax-only >> -verify %s >> >> // Test that we don't allow reinterpret_casts from pointers of one size >> to >> // pointers of a different size. >> @@ -12,3 +8,9 @@ >> >> void (A::*paf)(); >> void (C::*pcf)() = reinterpret_cast<void (C::*)()>(paf); >> //expected-error{{cannot reinterpret_cast from member pointer type}} >> + >> +class __single_inheritance D; >> +class __multiple_inheritance D; // expected-warning {{ignored since >> inheritance model was already declared as 'single'}} >> + >> +class __virtual_inheritance E; >> +class __virtual_inheritance E; // no warning expected since same >> attribute >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > -- João Matos
clang-MS-inheritance-warn-inherit.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
