https://github.com/MitalAshok created https://github.com/llvm/llvm-project/pull/91990
Fix `CXXRecordDecl::isParsingBaseSpecifiers` so that it is true while parsing base specifiers instead of directly after they have been parsed. -fcomplete-member-pointers now issues a diagnostic when a member pointer is used in a base specifier. -fcomplete-member-pointers has also been relaxed to not issue a diagnostic for incomplete classes with an explicit `__{single|multiple|virtual}_inheritance` attribute, whose completeness would not affect the representation of pointer-to-member objects. >From a5347082aa47a7aa525ece818f91fb6fdd5fdb0c Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Mon, 13 May 2024 16:59:06 +0100 Subject: [PATCH] [Clang] Fix Microsoft ABI inheritance model when member pointer is used in a base specifier Fix CXXRecordDecl::isParsingBaseSpecifiers so that it is true while parsing base specifiers instead of directly after they have been parsed. -fcomplete-member-pointers now issues a diagnostic when a member pointer is used in a base specifier. -fcomplete-member-pointers has also been relaxed to not issue a diagnostic for incomplete classes with an explicit __{single|multiple|virtual}_inheritance attribute, whose completeness would not affect the representation of pointer-to-member objects. --- clang/docs/ReleaseNotes.rst | 4 ++++ clang/include/clang/AST/DeclCXX.h | 5 ++-- .../clang/Basic/DiagnosticSemaKinds.td | 2 ++ clang/lib/AST/DeclCXX.cpp | 6 +---- clang/lib/Parse/ParseDeclCXX.cpp | 22 +++++++++++++++++ clang/lib/Sema/SemaDeclCXX.cpp | 3 --- clang/lib/Sema/SemaType.cpp | 23 +++++++++++------- .../test/SemaCXX/complete-member-pointers.cpp | 24 ++++++++++++++++++- clang/test/SemaCXX/member-pointer-ms.cpp | 8 +++++++ 9 files changed, 77 insertions(+), 20 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 4702b8c10cdbb..054332c2cee4e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -79,6 +79,10 @@ ABI Changes in This Version - Fixed Microsoft calling convention when returning classes that have a deleted copy assignment operator. Such a class should be returned indirectly. +- Fixed Microsoft layout of pointer-to-members of classes when the layout is needed + directly or indirectly by the base classes of a class. These should use the most + general unspecified inheritance layout. Also affects -fcomplete-member-pointers. + AST Dumping Potentially Breaking Changes ---------------------------------------- diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index fb52ac804849d..45b3b47ed51c5 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -297,7 +297,8 @@ class CXXRecordDecl : public RecordDecl { LLVM_PREFERRED_TYPE(bool) unsigned IsLambda : 1; - /// Whether we are currently parsing base specifiers. + /// Whether we are currently parsing base specifiers; the + /// colon has been consumed but the beginning left brace hasn't. LLVM_PREFERRED_TYPE(bool) unsigned IsParsingBaseSpecifiers : 1; @@ -598,7 +599,7 @@ class CXXRecordDecl : public RecordDecl { return !hasDefinition() || !isDynamicClass() || hasAnyDependentBases(); } - void setIsParsingBaseSpecifiers() { data().IsParsingBaseSpecifiers = true; } + void setIsParsingBaseSpecifiers(bool to = true) { data().IsParsingBaseSpecifiers = to; } bool isParsingBaseSpecifiers() const { return data().IsParsingBaseSpecifiers; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9e82130c93609..a9f9f02651cff 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8003,6 +8003,8 @@ def err_bad_memptr_rhs : Error< def err_bad_memptr_lhs : Error< "left hand operand to %0 must be a %select{|pointer to }1class " "compatible with the right hand operand, but is %2">; +def note_memptr_incomplete_until_bases : Note< + "this will affect the ABI of the member pointer until the bases have been specified">; def err_memptr_incomplete : Error< "member pointer has incomplete base type %0">; def warn_exception_caught_by_earlier_handler : Warning< diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 75c441293d62e..7f20a47e6a054 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -474,10 +474,8 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, if (data().IsStandardLayout && NumBases > 1 && hasRepeatedBaseClass(this)) data().IsStandardLayout = false; - if (VBases.empty()) { - data().IsParsingBaseSpecifiers = false; + if (VBases.empty()) return; - } // Create base specifier for any direct or indirect virtual bases. data().VBases = new (C) CXXBaseSpecifier[VBases.size()]; @@ -488,8 +486,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, addedClassSubobject(Type->getAsCXXRecordDecl()); data().getVBases()[I] = *VBases[I]; } - - data().IsParsingBaseSpecifiers = false; } unsigned CXXRecordDecl::getODRHash() const { diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 65ddebca49bc6..681b9803142c2 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -2334,6 +2334,26 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind, } } +namespace { + struct ParsingBaseSpecifiersGuard { + CXXRecordDecl *RD = nullptr; + ParsingBaseSpecifiersGuard(Sema& S, Decl *D) { + if (D) { + S.AdjustDeclIfTemplate(D); + RD = cast<CXXRecordDecl>(D); + assert(!RD->isParsingBaseSpecifiers() && "Recursively parsing base specifiers of the same class?"); + RD->setIsParsingBaseSpecifiers(true); + } + } + ~ParsingBaseSpecifiersGuard() { + if (RD) { + assert(RD->isParsingBaseSpecifiers() && "Stopped parsing base specifiers before exiting ParseBaseClause?"); + RD->setIsParsingBaseSpecifiers(false); + } + } + }; +} + /// ParseBaseClause - Parse the base-clause of a C++ class [C++ class.derived]. /// /// base-clause : [C++ class.derived] @@ -2345,6 +2365,8 @@ void Parser::ParseBaseClause(Decl *ClassDecl) { assert(Tok.is(tok::colon) && "Not a base clause"); ConsumeToken(); + ParsingBaseSpecifiersGuard Guard(Actions, ClassDecl); + // Build up an array of parsed base specifiers. SmallVector<CXXBaseSpecifier *, 8> BaseInfo; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 53238d355ea09..0bf7208605213 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -2862,9 +2862,6 @@ BaseResult Sema::ActOnBaseSpecifier(Decl *classdecl, SourceRange SpecifierRange, if (!Class) return true; - // We haven't yet attached the base specifiers. - Class->setIsParsingBaseSpecifiers(); - // We do not support any C++11 attributes on base-specifiers yet. // Diagnose any attributes we see. for (const ParsedAttr &AL : Attributes) { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index fddc3545ecb61..0a801f1797d23 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -9463,17 +9463,22 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>()) { if (!MPTy->getClass()->isDependentType()) { - if (getLangOpts().CompleteMemberPointers && - !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() && - RequireCompleteType(Loc, QualType(MPTy->getClass(), 0), Kind, - diag::err_memptr_incomplete)) - return true; - // We lock in the inheritance model once somebody has asked us to ensure // that a pointer-to-member type is complete. - if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { - (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0)); - assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl()); + if (Context.getTargetInfo().getCXXABI().isMicrosoft() || getLangOpts().CompleteMemberPointers) { + QualType Class = QualType(MPTy->getClass(), 0); + (void)isCompleteType(Loc, Class); + CXXRecordDecl *RD = MPTy->getMostRecentCXXRecordDecl(); + assignInheritanceModel(*this, RD); + + if (getLangOpts().CompleteMemberPointers && RD->getMSInheritanceModel() == MSInheritanceModel::Unspecified) { + if (RD->hasDefinition() && RD->isParsingBaseSpecifiers()) { + Diag(Loc, diag::err_memptr_incomplete) << Class; + Diag(RD->getDefinition()->getLocation(), diag::note_memptr_incomplete_until_bases); + } else if (!RequireCompleteType(Loc, Class, diag::err_memptr_incomplete)) + Diag(Loc, diag::err_memptr_incomplete) << Class; + return true; + } } } } diff --git a/clang/test/SemaCXX/complete-member-pointers.cpp b/clang/test/SemaCXX/complete-member-pointers.cpp index 942bb703a9223..41c9b98438c1a 100644 --- a/clang/test/SemaCXX/complete-member-pointers.cpp +++ b/clang/test/SemaCXX/complete-member-pointers.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -verify -fsyntax-only -fcomplete-member-pointers %s +// RUN: %clang_cc1 -verify -fsyntax-only -fc++-abi=itanium -fms-extensions -fcomplete-member-pointers %s +// RUN: %clang_cc1 -verify -fsyntax-only -triple=x86_64-unknown-win32 -fc++-abi=microsoft -fms-compatibility -fcomplete-member-pointers %s +// RUN: %clang_cc1 -verify -fsyntax-only -triple=x86_64-unknown-win32 -fc++-abi=itanium -fms-compatibility -fcomplete-member-pointers %s struct S; // expected-note {{forward declaration of 'S'}} typedef int S::*t; @@ -13,3 +15,23 @@ template <typename T> struct S3 { int T::*foo; }; + +struct __single_inheritance S4; +int S4::* baz; + +template<int I> struct Base {}; +struct __single_inheritance S5 : Base<sizeof(int S5::*)> {}; +struct +S6 // #S6 +: +Base<sizeof(int S6::*)> +// expected-error@-1 {{member pointer has incomplete base type 'S6'}} +// expected-note@#S6 {{this will affect the ABI of the member pointer until the bases have been specified}} +{ +}; + +template<typename T> struct S7 {}; +int S7<void>::* qux; // #qux +template<> struct S7<void> {}; +// expected-error@-1 {{explicit specialization of 'S7<void>' after instantiation}} +// expected-note@#qux {{implicit instantiation first required here}} diff --git a/clang/test/SemaCXX/member-pointer-ms.cpp b/clang/test/SemaCXX/member-pointer-ms.cpp index 3271ff0c623a2..efbab8b28498f 100644 --- a/clang/test/SemaCXX/member-pointer-ms.cpp +++ b/clang/test/SemaCXX/member-pointer-ms.cpp @@ -215,6 +215,14 @@ struct NewUnspecified; SingleTemplate<void (IncSingle::*)()> tmpl_single; UnspecTemplate<void (NewUnspecified::*)()> tmpl_unspec; +// Member pointers used in base specifiers force an unspecified inheritance model +struct MemPtrInBase : UnspecTemplate<void (MemPtrInBase::*)()> {}; +#ifdef VMB +struct __single_inheritance SpecifiedMemPtrInBase; +struct SpecifiedMemPtrInBase : SingleTemplate<void (SpecifiedMemPtrInBase::*)()> {}; +struct __single_inheritance SpecifiedMemPtrInBase2 : SingleTemplate<void (SpecifiedMemPtrInBase2::*)()> {}; +#endif + struct NewUnspecified { }; static_assert(sizeof(void (NewUnspecified::*)()) == kUnspecifiedFunctionSize, ""); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits