[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

[Tom Honermann via Phabricator via cfe-commits]

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 at VA@PMFInTemplateArgument@@$1?printd at 12@AAEHH at 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158869.554413.patch
Type: text/x-patch
Size: 3478 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230829/c22fdc10/attachment-0001.bin>