[Clang] Fix Microsoft ABI inheritance model when member pointer is used in a base specifier

clang/docs/ReleaseNotes.rst

@@ -113,6 +113,10 @@ ABI Changes in This Version
   `-fms-compatibility-version=19.14` to imitate the MSVC 1914 mangling behavior.
   (GH#70899).
 
+- 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
 ----------------------------------------
 

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) {
+    data().IsParsingBaseSpecifiers = To;
+  }
 
   bool isParsingBaseSpecifiers() const {
     return data().IsParsingBaseSpecifiers;

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,

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 {

clang/lib/Parse/ParseDeclCXX.cpp

@@ -2375,6 +2375,17 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
   }
 }
 
+namespace {
+struct ParsingBaseSpecifiersGuard {
+  Sema &Actions;
+  Decl *D = nullptr;
+  ParsingBaseSpecifiersGuard(Sema &Actions, Decl *D) : Actions(Actions), D(D) {
+    Actions.SetParsingBaseSpecifiers(D, true);
+  }
+  ~ParsingBaseSpecifiersGuard() { Actions.SetParsingBaseSpecifiers(D, false); }
+};
+} // namespace
+
 /// ParseBaseClause - Parse the base-clause of a C++ class [C++ class.derived].
 ///
 ///       base-clause : [C++ class.derived]
@@ -2386,6 +2397,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;
 

clang/lib/Sema/SemaDeclCXX.cpp

@@ -2763,9 +2763,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) {
@@ -2829,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())

clang/lib/Sema/SemaType.cpp

@@ -9015,11 +9015,20 @@ 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;
+      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.

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,18 @@ template <typename T>
 struct S3 {
   int T::*foo;
 };
+
+template<int I> struct Base {};
+struct
+S5 // #S5
+:
+Base<sizeof(int S5::*)>
+// expected-error@-1 {{member pointer has incomplete base type 'S5'}}
+{
+};
+
+template<typename T> struct S6 {};
+int S6<void>::* qux;  // #qux
+template<> struct S6<void> {};
+// expected-error@-1 {{explicit specialization of 'S6<void>' after instantiation}}
+//   expected-note@#qux {{implicit instantiation first required here}}

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, "");