[clang] [Clang] Fix Microsoft ABI inheritance model when member pointer is used in a base specifier (PR #91990)
Mital Ashok via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 6 02:59:09 PDT 2024
https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/91990
>From 5dc9193af0d98335a87e93ad70d945dbc0ffce79 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Mon, 13 May 2024 16:59:06 +0100
Subject: [PATCH 1/5] [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 | 7 +++--
.../clang/Basic/DiagnosticSemaKinds.td | 2 ++
clang/lib/AST/DeclCXX.cpp | 6 +----
clang/lib/Parse/ParseDeclCXX.cpp | 24 +++++++++++++++++
clang/lib/Sema/SemaDeclCXX.cpp | 3 ---
clang/lib/Sema/SemaType.cpp | 27 ++++++++++++-------
.../test/SemaCXX/complete-member-pointers.cpp | 24 ++++++++++++++++-
clang/test/SemaCXX/member-pointer-ms.cpp | 8 ++++++
9 files changed, 85 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..81669b1f606b3 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,9 @@ 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..a28503b5b4de4 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2334,6 +2334,28 @@ 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);
+ }
+ }
+};
+} // namespace
+
/// ParseBaseClause - Parse the base-clause of a C++ class [C++ class.derived].
///
/// base-clause : [C++ class.derived]
@@ -2345,6 +2367,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..e00ee46c9d78d 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9463,17 +9463,26 @@ 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 at -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 at -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, "");
>From f3436707c24abac102a4f68e01b5dd1fe6dc334c Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Tue, 11 Jun 2024 15:19:12 +0100
Subject: [PATCH 2/5] Add more places where -fcomplete-member-pointers should
match the Microsoft ABI
---
clang/lib/Sema/SemaCast.cpp | 6 ++++--
clang/lib/Sema/SemaExpr.cpp | 9 ++++++---
clang/lib/Sema/SemaExprCXX.cpp | 3 ++-
clang/lib/Sema/SemaOverload.cpp | 3 ++-
clang/lib/Sema/SemaType.cpp | 3 ++-
5 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index f03dcf05411df..ba5ce20a25614 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1795,7 +1795,8 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
// Lock down the inheritance model right now in MS ABI, whether or not the
// pointee types are the same.
- if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ Self.getLangOpts().CompleteMemberPointers) {
(void)Self.isCompleteType(OpRange.getBegin(), SrcType);
(void)Self.isCompleteType(OpRange.getBegin(), DestType);
}
@@ -2337,7 +2338,8 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
SrcMemPtr->isMemberFunctionPointer())
return TC_NotApplicable;
- if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ Self.getLangOpts().CompleteMemberPointers) {
// We need to determine the inheritance model that the class will use if
// haven't yet.
(void)Self.isCompleteType(OpRange.getBegin(), SrcType);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 76145f291887c..cd93c0a75ec49 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -728,7 +728,8 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
// Under the MS ABI, lock down the inheritance model now.
if (T->isMemberPointerType() &&
- Context.getTargetInfo().getCXXABI().isMicrosoft())
+ (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers))
(void)isCompleteType(E->getExprLoc(), T);
ExprResult Res = CheckLValueToRValueConversionOperand(E);
@@ -14208,7 +14209,8 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
QualType MPTy = Context.getMemberPointerType(
op->getType(), Context.getTypeDeclType(MD->getParent()).getTypePtr());
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers)
(void)isCompleteType(OpLoc, MPTy);
return MPTy;
} else if (lval != Expr::LV_Valid && lval != Expr::LV_IncompleteVoidType) {
@@ -14288,7 +14290,8 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
op->getType(),
Context.getTypeDeclType(cast<RecordDecl>(Ctx)).getTypePtr());
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers)
(void)isCompleteType(OpLoc, MPTy);
return MPTy;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f3af8dee6b090..78e25500cc099 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4679,7 +4679,8 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
// We may not have been able to figure out what this member pointer resolved
// to up until this exact point. Attempt to lock-in it's inheritance model.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers) {
(void)isCompleteType(From->getExprLoc(), From->getType());
(void)isCompleteType(From->getExprLoc(), ToType);
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 1b4bcdcb51160..0532aa0bbb497 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -16442,7 +16442,8 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found,
QualType MemPtrType
= Context.getMemberPointerType(Fn->getType(), ClassType.getTypePtr());
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers)
(void)isCompleteType(UnOp->getOperatorLoc(), MemPtrType);
return UnaryOperator::Create(Context, SubExpr.get(), UO_AddrOf,
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 6b1ab4a27ba82..c403dbf0124d5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -2117,7 +2117,8 @@ QualType Sema::BuildArrayType(QualType T, ArraySizeModifier ASM,
// Mentioning a member pointer type for an array type causes us to lock in
// an inheritance model, even if it's inside an unused typedef.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft())
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+ getLangOpts().CompleteMemberPointers)
if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>())
if (!MPTy->getClass()->isDependentType())
(void)isCompleteType(Loc, T);
>From 66c865817901939f6251b28f87a0994ab684ee5e Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 6 Jul 2024 09:49:34 +0100
Subject: [PATCH 3/5] Revert "Add more places where -fcomplete-member-pointers
should match the Microsoft ABI"
This reverts commit f3436707c24abac102a4f68e01b5dd1fe6dc334c.
I'm planning on moving this to a separate pr which substantially changes -fcomplete-member-pointers
---
clang/lib/Sema/SemaCast.cpp | 6 ++----
clang/lib/Sema/SemaExpr.cpp | 9 +++------
clang/lib/Sema/SemaExprCXX.cpp | 3 +--
clang/lib/Sema/SemaOverload.cpp | 3 +--
clang/lib/Sema/SemaType.cpp | 3 +--
5 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 5b82ca2e44c3c..eca8363ee9605 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1793,8 +1793,7 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
// Lock down the inheritance model right now in MS ABI, whether or not the
// pointee types are the same.
- if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- Self.getLangOpts().CompleteMemberPointers) {
+ if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
(void)Self.isCompleteType(OpRange.getBegin(), SrcType);
(void)Self.isCompleteType(OpRange.getBegin(), DestType);
}
@@ -2336,8 +2335,7 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
SrcMemPtr->isMemberFunctionPointer())
return TC_NotApplicable;
- if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- Self.getLangOpts().CompleteMemberPointers) {
+ if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
// We need to determine the inheritance model that the class will use if
// haven't yet.
(void)Self.isCompleteType(OpRange.getBegin(), SrcType);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7566a73346303..852344d895ffd 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -710,8 +710,7 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
// Under the MS ABI, lock down the inheritance model now.
if (T->isMemberPointerType() &&
- (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers))
+ Context.getTargetInfo().getCXXABI().isMicrosoft())
(void)isCompleteType(E->getExprLoc(), T);
ExprResult Res = CheckLValueToRValueConversionOperand(E);
@@ -14066,8 +14065,7 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
}
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers)
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft())
(void)isCompleteType(OpLoc, MPTy);
return MPTy;
} else if (lval != Expr::LV_Valid && lval != Expr::LV_IncompleteVoidType) {
@@ -14147,8 +14145,7 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
op->getType(),
Context.getTypeDeclType(cast<RecordDecl>(Ctx)).getTypePtr());
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers)
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft())
(void)isCompleteType(OpLoc, MPTy);
return MPTy;
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d5fe23e061d92..fcf2189a308a8 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4607,8 +4607,7 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
// We may not have been able to figure out what this member pointer resolved
// to up until this exact point. Attempt to lock-in it's inheritance model.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers) {
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
(void)isCompleteType(From->getExprLoc(), From->getType());
(void)isCompleteType(From->getExprLoc(), ToType);
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c670a1f3b596..5ea6b06121c7c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -16193,8 +16193,7 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found,
QualType MemPtrType
= Context.getMemberPointerType(Fn->getType(), ClassType.getTypePtr());
// Under the MS ABI, lock down the inheritance model now.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers)
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft())
(void)isCompleteType(UnOp->getOperatorLoc(), MemPtrType);
return UnaryOperator::Create(Context, SubExpr.get(), UO_AddrOf,
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index a12bef92a6d27..215e4e410fccf 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -2056,8 +2056,7 @@ QualType Sema::BuildArrayType(QualType T, ArraySizeModifier ASM,
// Mentioning a member pointer type for an array type causes us to lock in
// an inheritance model, even if it's inside an unused typedef.
- if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
- getLangOpts().CompleteMemberPointers)
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft())
if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>())
if (!MPTy->getClass()->isDependentType())
(void)isCompleteType(Loc, T);
>From 7058fb49e900786744cd7d2d64a1af1004022e6f Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 6 Jul 2024 10:38:40 +0100
Subject: [PATCH 4/5] Partially revert changes to -fcomplete-member-pointers
I'm planning on moving this to a separate pr which substantially changes -fcomplete-member-pointers
---
.../clang/Basic/DiagnosticSemaKinds.td | 2 --
clang/lib/Sema/SemaType.cpp | 36 +++++++++----------
.../test/SemaCXX/complete-member-pointers.cpp | 19 ++++------
3 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index cf6fcc23a1d84..44fd51ec9abc9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8082,8 +8082,6 @@ 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/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 215e4e410fccf..5b803112ffead 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9015,26 +9015,26 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>()) {
if (!MPTy->getClass()->isDependentType()) {
+ if (getLangOpts().CompleteMemberPointers) {
+ const CXXRecordDecl *RD = MPTy->getClass()->getAsCXXRecordDecl();
+ if (RD->isBeingDefined()) {
+ if (RD->isParsingBaseSpecifiers()) {
+ Diag(Loc, diag::err_memptr_incomplete)
+ << QualType(MPTy->getClass(), 0);
+ return true;
+ }
+ // Otherwise, we know the bases and can assign a proper inheritance
+ // model even though the class is incomplete.
+ } else if (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() ||
- 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;
- }
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
+ assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
}
}
}
diff --git a/clang/test/SemaCXX/complete-member-pointers.cpp b/clang/test/SemaCXX/complete-member-pointers.cpp
index 41c9b98438c1a..3b2c0167e05e6 100644
--- a/clang/test/SemaCXX/complete-member-pointers.cpp
+++ b/clang/test/SemaCXX/complete-member-pointers.cpp
@@ -16,22 +16,17 @@ 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
+S5 // #S5
:
-Base<sizeof(int S6::*)>
-// expected-error at -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}}
+Base<sizeof(int S5::*)>
+// expected-error at -1 {{member pointer has incomplete base type 'S5'}}
{
};
-template<typename T> struct S7 {};
-int S7<void>::* qux; // #qux
-template<> struct S7<void> {};
-// expected-error at -1 {{explicit specialization of 'S7<void>' after instantiation}}
+template<typename T> struct S6 {};
+int S6<void>::* qux; // #qux
+template<> struct S6<void> {};
+// expected-error at -1 {{explicit specialization of 'S6<void>' after instantiation}}
// expected-note@#qux {{implicit instantiation first required here}}
>From 7c619ab5a0ddf3126cafe60099794eb8c22674b5 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 6 Jul 2024 10:58:37 +0100
Subject: [PATCH 5/5] Manipulate the CXXRecordDecl in Sema instead of Parse
---
clang/include/clang/AST/DeclCXX.h | 4 ++--
clang/include/clang/Sema/Sema.h | 4 ++++
clang/lib/Parse/ParseDeclCXX.cpp | 21 +++++----------------
clang/lib/Sema/SemaDeclCXX.cpp | 10 ++++++++++
4 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 81669b1f606b3..4a477897ce429 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -599,8 +599,8 @@ class CXXRecordDecl : public RecordDecl {
return !hasDefinition() || !isDynamicClass() || hasAnyDependentBases();
}
- void setIsParsingBaseSpecifiers(bool to = true) {
- data().IsParsingBaseSpecifiers = to;
+ void setIsParsingBaseSpecifiers(bool To) {
+ data().IsParsingBaseSpecifiers = To;
}
bool isParsingBaseSpecifiers() const {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 75a80540dbcbf..ad140694faed5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5651,6 +5651,10 @@ class Sema final : public SemaBase {
SourceLocation BaseLoc,
SourceLocation EllipsisLoc);
+ /// Sets the value of "isParsingBaseSpecifiers" when parsing a
+ /// C++ class definition.
+ void SetParsingBaseSpecifiers(Decl *ClassDecl, bool To);
+
/// Performs the actual work of attaching the given base class
/// specifiers to a C++ class.
bool AttachBaseSpecifiers(CXXRecordDecl *Class,
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 52e77afe6b042..de4f2d6b1a9f3 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2377,23 +2377,12 @@ 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);
- }
+ Sema &Actions;
+ Decl *D = nullptr;
+ ParsingBaseSpecifiersGuard(Sema &Actions, Decl *D) : Actions(Actions), D(D) {
+ Actions.SetParsingBaseSpecifiers(D, true);
}
+ ~ParsingBaseSpecifiersGuard() { Actions.SetParsingBaseSpecifiers(D, false); }
};
} // namespace
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5567b6d3d37c4..39ce2e2fa9704 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2826,6 +2826,16 @@ NoteIndirectBases(ASTContext &Context, IndirectBaseSet &Set,
}
}
+void Sema::SetParsingBaseSpecifiers(Decl *ClassDecl, bool To) {
+ if (ClassDecl) {
+ AdjustDeclIfTemplate(ClassDecl);
+ auto *RD = cast<CXXRecordDecl>(ClassDecl);
+ assert(RD->isParsingBaseSpecifiers() != To &&
+ "Unexpected state for isParsingBaseSpecifiers()");
+ RD->setIsParsingBaseSpecifiers(To);
+ }
+}
+
bool Sema::AttachBaseSpecifiers(CXXRecordDecl *Class,
MutableArrayRef<CXXBaseSpecifier *> Bases) {
if (Bases.empty())
More information about the cfe-commits
mailing list