adr26 updated this revision to Diff 149300.
adr26 added a comment.
Hi Reid,
I've added testcases matching the issues I hit in
`microsoft-abi-member-pointers.cpp` (no change to the rest of the fix). If you
are happy with this, please feel free to push.
I checked with MSVC, and as per the added testcases, it appears to always lock
in the inheritance model as virutal when there's a member funtion pointer in a
dependant base, irrespective of whether the most-derived class ends up using
single/multiple/virtual inheritance. In fact if, for example, you annotate
DerivedFunctor (in the test I've added) using an MS inheritance keyword - i.e.
as "class __single_inheritance DerviceFunctor" - then MSVC complains that it's
already fixed the representation to be virtual:
C:\Temp\clang\clang>cl
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26428.1 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
microsoft-abi-member-pointers.cpp
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(81):
error C2286: pointers to members of 'pr37399::DerivedFunctor<void>'
representation is already set to virtual_inheritance - declaration ignored
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(27):
note: see declaration of 'pr37399::DerivedFunctor<void>'
as such, these tests cover the cases I was hitting and appear to have
behavioural parity with MSVC (as is the intention!).
Thanks, Andrew R
https://reviews.llvm.org/D46664
Files:
include/clang/AST/DeclCXX.h
lib/AST/MicrosoftMangle.cpp
lib/AST/Type.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/Sema/SemaType.cpp
test/CodeGenCXX/microsoft-abi-member-pointers.cpp
Index: test/CodeGenCXX/microsoft-abi-member-pointers.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-member-pointers.cpp
+++ test/CodeGenCXX/microsoft-abi-member-pointers.cpp
@@ -3,6 +3,124 @@
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -fms-extensions -verify
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -DMEMFUN -fms-extensions -verify
+namespace pr37399 {
+template <typename T>
+struct Functor {
+ void (T::*PtrToMemberFunction)();
+};
+// CHECK-DAG: %"struct.pr37399::Functor" = type { i8* }
+
+template <typename SomeType>
+class SimpleDerivedFunctor;
+template <typename SomeType>
+class SimpleDerivedFunctor : public Functor<SimpleDerivedFunctor<SomeType>> {};
+// CHECK-DAG: %"class.pr37399::SimpleDerivedFunctor" = type { %"struct.pr37399::Functor" }
+
+SimpleDerivedFunctor<void> SimpleFunctor;
+// CHECK-DAG: @"?SimpleFunctor@pr37399@@3V?$SimpleDerivedFunctor@X@1@A" = dso_local global %"class.pr37399::SimpleDerivedFunctor" zeroinitializer, align 4
+
+short Global = 0;
+template <typename SomeType>
+class DerivedFunctor;
+template <typename SomeType>
+class DerivedFunctor
+ : public Functor<DerivedFunctor<void>> {
+public:
+ void Foo() {
+ Global = 42;
+ }
+};
+
+class MultipleBase {
+public:
+ MultipleBase() : Value() {}
+ short Value;
+};
+// CHECK-DAG: %"class.pr37399::MultipleBase" = type { i16 }
+
+template <typename SomeType>
+class MultiplyDerivedFunctor;
+template <typename SomeType>
+class MultiplyDerivedFunctor
+ : public Functor<MultiplyDerivedFunctor<void>>,
+ public MultipleBase {
+public:
+ void Foo() {
+ MultipleBase::Value = 42*2;
+ }
+};
+
+class VirtualBase {
+public:
+ VirtualBase() : Value() {}
+ short Value;
+};
+// CHECK-DAG: %"class.pr37399::VirtualBase" = type { i16 }
+
+template <typename SomeType>
+class VirtBaseFunctor
+ : public Functor<SomeType>,
+ public virtual VirtualBase{};
+template <typename SomeType>
+class VirtuallyDerivedFunctor;
+template <typename SomeType>
+class VirtuallyDerivedFunctor
+ : public VirtBaseFunctor<VirtuallyDerivedFunctor<void>>,
+ public virtual VirtualBase {
+public:
+ void Foo() {
+ VirtualBase::Value = 42*3;
+ }
+};
+} // namespace pr37399
+
+pr37399::DerivedFunctor<int> BFunctor;
+// CHECK-DAG: @"?BFunctor@@3V?$DerivedFunctor@H@pr37399@@A" = dso_local global %"[[BFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[BFUNCTOR]]" = type { %"[[BFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" }
+// CHECK-DAG: %"[[BFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+pr37399::DerivedFunctor<void> AFunctor;
+// CHECK-DAG: @"?AFunctor@@3V?$DerivedFunctor@X@pr37399@@A" = dso_local global %"[[AFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[AFUNCTOR]]" = type { %"[[AFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" }
+// CHECK-DAG: %"[[AFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+
+pr37399::MultiplyDerivedFunctor<int> DFunctor;
+// CHECK-DAG: @"?DFunctor@@3V?$MultiplyDerivedFunctor@H@pr37399@@A" = dso_local global %"[[DFUNCTOR:class.pr37399::MultiplyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[DFUNCTOR]]" = type { %"[[DFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]", %"class.pr37399::MultipleBase", [6 x i8] }
+// CHECK-DAG: %"[[DFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+pr37399::MultiplyDerivedFunctor<void> CFunctor;
+// CHECK-DAG: @"?CFunctor@@3V?$MultiplyDerivedFunctor@X@pr37399@@A" = dso_local global %"[[CFUNCTOR:class.pr37399::MultiplyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[CFUNCTOR]]" = type { %"[[CFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]", %"class.pr37399::MultipleBase", [6 x i8] }
+// CHECK-DAG: %"[[CFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+
+pr37399::VirtuallyDerivedFunctor<int> FFunctor;
+// CHECK-DAG: @"?FFunctor@@3V?$VirtuallyDerivedFunctor@H@pr37399@@A" = dso_local global %"[[FFUNCTOR:class.pr37399::VirtuallyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[FFUNCTOR]]" = type { %"class.pr37399::VirtBaseFunctor.base", %"class.pr37399::VirtualBase" }
+pr37399::VirtuallyDerivedFunctor<void> EFunctor;
+// CHECK-DAG: @"?EFunctor@@3V?$VirtuallyDerivedFunctor@X@pr37399@@A" = dso_local global %"[[EFUNCTOR:class.pr37399::VirtuallyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[EFUNCTOR]]" = type { %"class.pr37399::VirtBaseFunctor.base", %"class.pr37399::VirtualBase" }
+
+// CHECK-DAG: %"class.pr37399::VirtBaseFunctor.base" = type <{ %"[[VFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]", i32*, [4 x i8] }>
+// CHECK-DAG: %"[[VFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+
+namespace pr37399 {
+void SingleInheritanceFnPtrCall() {
+ BFunctor.PtrToMemberFunction = &DerivedFunctor<void>::Foo;
+ (AFunctor.*(BFunctor.PtrToMemberFunction))();
+}
+void MultipleInheritanceFnPtrCall() {
+ DFunctor.PtrToMemberFunction = &MultiplyDerivedFunctor<void>::Foo;
+ Global = CFunctor.MultipleBase::Value;
+ (CFunctor.*(DFunctor.PtrToMemberFunction))();
+ Global = CFunctor.MultipleBase::Value;
+}
+void VirtualInheritanceFnPtrCall() {
+ FFunctor.PtrToMemberFunction = &VirtuallyDerivedFunctor<void>::Foo;
+ Global = EFunctor.VirtualBase::Value;
+ (EFunctor.*(FFunctor.PtrToMemberFunction))();
+ Global = EFunctor.VirtualBase::Value;
+}
+} // namespace pr37399
+
struct PR26313_Y;
typedef void (PR26313_Y::*PR26313_FUNC)();
struct PR26313_X {
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7544,7 +7544,7 @@
/// Locks in the inheritance model for the given class and all of its bases.
static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) {
- RD = RD->getMostRecentDecl();
+ RD = RD->getMostRecentNonInjectedDecl();
if (!RD->hasAttr<MSInheritanceAttr>()) {
MSInheritanceAttr::Spelling IM;
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2733,7 +2733,7 @@
assert(MD->isInstance() && "Member function must not be static!");
CharUnits NonVirtualBaseAdjustment = CharUnits::Zero();
- const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl();
+ const CXXRecordDecl *RD = MD->getParent()->getMostRecentNonInjectedDecl();
CodeGenTypes &Types = CGM.getTypes();
unsigned VBTableIndex = 0;
Index: lib/AST/Type.cpp
===================================================================
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2040,7 +2040,7 @@
return false;
// The inheritance attribute might only be present on the most recent
// CXXRecordDecl, use that one.
- RD = RD->getMostRecentDecl();
+ RD = RD->getMostRecentNonInjectedDecl();
// Nothing interesting to do if the inheritance attribute is already set.
if (RD->hasAttr<MSInheritanceAttr>())
return false;
@@ -3936,5 +3936,5 @@
}
CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const {
- return getClass()->getAsCXXRecordDecl()->getMostRecentDecl();
+ return getClass()->getAsCXXRecordDecl()->getMostRecentNonInjectedDecl();
}
Index: lib/AST/MicrosoftMangle.cpp
===================================================================
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -1370,12 +1370,12 @@
const NamedDecl *ND = TA.getAsDecl();
if (isa<FieldDecl>(ND) || isa<IndirectFieldDecl>(ND)) {
mangleMemberDataPointer(
- cast<CXXRecordDecl>(ND->getDeclContext())->getMostRecentDecl(),
+ cast<CXXRecordDecl>(ND->getDeclContext())->getMostRecentNonInjectedDecl(),
cast<ValueDecl>(ND));
} else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
if (MD && MD->isInstance()) {
- mangleMemberFunctionPointer(MD->getParent()->getMostRecentDecl(), MD);
+ mangleMemberFunctionPointer(MD->getParent()->getMostRecentNonInjectedDecl(), MD);
} else {
Out << "$1?";
mangleName(FD);
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -751,6 +751,21 @@
return const_cast<CXXRecordDecl*>(this)->getMostRecentDecl();
}
+ CXXRecordDecl *getMostRecentNonInjectedDecl() {
+ CXXRecordDecl *Recent =
+ static_cast<CXXRecordDecl *>(this)->getMostRecentDecl();
+ while (Recent->isInjectedClassName()) {
+ // FIXME: Does injected class name need to be in the redeclarations chain?
+ assert(Recent->getPreviousDecl());
+ Recent = Recent->getPreviousDecl();
+ }
+ return Recent;
+ }
+
+ const CXXRecordDecl *getMostRecentNonInjectedDecl() const {
+ return const_cast<CXXRecordDecl*>(this)->getMostRecentNonInjectedDecl();
+ }
+
CXXRecordDecl *getDefinition() const {
// We only need an update if we don't already know which
// declaration is the definition.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
