tahonermann updated this revision to Diff 554413.
tahonermann marked an inline comment as done.
tahonermann edited the summary of this revision.
tahonermann added a comment.
Address prior review feedback.
This update also modifies the propagation of the `MSInheritanceAttr` attribute
for explicit template instantiations to clone the attribute and to set it as
inherited. Previously, the same `MSInheritanceAttr` object was being attached
to each declaration. @erichkeane, it would be great if you could confirm that
this is the correct behavior.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158869/new/
https://reviews.llvm.org/D158869
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
===================================================================
--- clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-extensions | FileCheck -allow-deprecated-dag-overlap
%s
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o -
-triple=x86_64-pc-win32 -fms-extensions | FileCheck %s -check-prefix=X64
+// RUN: %clang_cc1 -std=c++17 -Wno-uninitialized -fno-rtti -emit-llvm %s -o -
-triple=x86_64-pc-win32 -fms-extensions | FileCheck %s -check-prefix=X64
// 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
@@ -934,3 +935,32 @@
void A::printd() { JSMethod<A, &A::printd>(); }
// CHECK-LABEL:
@"??$JSMethod@VA@PMFInTemplateArgument@@$1?printd@12@AAEHH@Z@PMFInTemplateArgument@@YAXXZ"(
}
+
+namespace MissingMSInheritanceAttr {
+// This is a regression test for an assertion failure that occurred when
+// compiling for C++17. The issue concerned a failure to propagate a
+// MSInheritanceAttr attribute for the explicit template instantiation
+// definition prior to it being required to complete the specialization
+// definition in order to determine its alignment so as to resolve a
+// lookup for a deallocation function for the virtual destructor.
+template <typename>
+class a;
+struct b {
+ typedef void (a<int>::*f)();
+ f d;
+};
+template <typename>
+class a {
+ virtual ~a();
+ b e;
+};
+extern template class a<int>;
+template class a<int>;
+#ifdef _WIN64
+static_assert(sizeof(b::d) == 24, "");
+static_assert(sizeof(void (a<int>::*)()) == 24, "");
+#else
+static_assert(sizeof(b::d) == 16, "");
+static_assert(sizeof(void (a<int>::*)()) == 16, "");
+#endif
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -10105,6 +10105,17 @@
ClassTemplate, CanonicalConverted, PrevDecl);
SetNestedNameSpecifier(*this, Specialization, SS);
+ // A MSInheritanceAttr attached to the previous declaration must be
+ // propagated to the new node prior to instantiation.
+ if (PrevDecl) {
+ if (const auto *A = PrevDecl->getAttr<MSInheritanceAttr>()) {
+ auto *Clone = A->clone(getASTContext());
+ Clone->setInherited(true);
+ Specialization->addAttr(Clone);
+ Consumer.AssignInheritanceModel(Specialization);
+ }
+ }
+
if (!HasNoEffect && !PrevDecl) {
// Insert the new specialization.
ClassTemplate->AddSpecialization(Specialization, InsertPos);
@@ -10221,11 +10232,6 @@
dllExportImportClassTemplateSpecialization(*this, Def);
}
- if (Def->hasAttr<MSInheritanceAttr>()) {
- Specialization->addAttr(Def->getAttr<MSInheritanceAttr>());
- Consumer.AssignInheritanceModel(Specialization);
- }
-
// Set the template specialization kind. Make sure it is set before
// instantiating the members which will trigger ASTConsumer callbacks.
Specialization->setTemplateSpecializationKind(TSK);
Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
===================================================================
--- clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -fms-extensions | FileCheck -allow-deprecated-dag-overlap %s
// RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s -check-prefix=X64
+// RUN: %clang_cc1 -std=c++17 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s -check-prefix=X64
// 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
@@ -934,3 +935,32 @@
void A::printd() { JSMethod<A, &A::printd>(); }
// CHECK-LABEL: @"??$JSMethod@VA@PMFInTemplateArgument@@$1?printd@12@AAEHH@Z@PMFInTemplateArgument@@YAXXZ"(
}
+
+namespace MissingMSInheritanceAttr {
+// This is a regression test for an assertion failure that occurred when
+// compiling for C++17. The issue concerned a failure to propagate a
+// MSInheritanceAttr attribute for the explicit template instantiation
+// definition prior to it being required to complete the specialization
+// definition in order to determine its alignment so as to resolve a
+// lookup for a deallocation function for the virtual destructor.
+template <typename>
+class a;
+struct b {
+ typedef void (a<int>::*f)();
+ f d;
+};
+template <typename>
+class a {
+ virtual ~a();
+ b e;
+};
+extern template class a<int>;
+template class a<int>;
+#ifdef _WIN64
+static_assert(sizeof(b::d) == 24, "");
+static_assert(sizeof(void (a<int>::*)()) == 24, "");
+#else
+static_assert(sizeof(b::d) == 16, "");
+static_assert(sizeof(void (a<int>::*)()) == 16, "");
+#endif
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -10105,6 +10105,17 @@
ClassTemplate, CanonicalConverted, PrevDecl);
SetNestedNameSpecifier(*this, Specialization, SS);
+ // A MSInheritanceAttr attached to the previous declaration must be
+ // propagated to the new node prior to instantiation.
+ if (PrevDecl) {
+ if (const auto *A = PrevDecl->getAttr<MSInheritanceAttr>()) {
+ auto *Clone = A->clone(getASTContext());
+ Clone->setInherited(true);
+ Specialization->addAttr(Clone);
+ Consumer.AssignInheritanceModel(Specialization);
+ }
+ }
+
if (!HasNoEffect && !PrevDecl) {
// Insert the new specialization.
ClassTemplate->AddSpecialization(Specialization, InsertPos);
@@ -10221,11 +10232,6 @@
dllExportImportClassTemplateSpecialization(*this, Def);
}
- if (Def->hasAttr<MSInheritanceAttr>()) {
- Specialization->addAttr(Def->getAttr<MSInheritanceAttr>());
- Consumer.AssignInheritanceModel(Specialization);
- }
-
// Set the template specialization kind. Make sure it is set before
// instantiating the members which will trigger ASTConsumer callbacks.
Specialization->setTemplateSpecializationKind(TSK);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
