[cfe-commits] r163078 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/MicrosoftCXXABI.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaType.cpp test/SemaCXX/member-pointer-ms.cpp

João Matos ripzonetriton at gmail.com
Tue Sep 4 09:20:21 PDT 2012


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 <chandlerc at google.com>wrote:

> On Sat, Sep 1, 2012 at 8:13 PM, Joao Matos <ripzonetriton at gmail.com>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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>


-- 
João Matos
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120904/a1801e49/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-MS-inheritance-warn-inherit.patch
Type: application/octet-stream
Size: 7472 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120904/a1801e49/attachment.obj>


More information about the cfe-commits mailing list