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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change addresses two issues:

1. A failure to propagate a `MSInheritanceAttr` prior to it being required by an explicit class template instantiation definition.
2. The same `MSInheritanceAttr` attribute being attached to the same `ClassTemplateSpecializationDecl` twice.

`Sema::ActOnExplicitInstantiation()` is responsible for the construction of `ClassTemplateSpecializationDecl` nodes for explicit template instantiation declarations and definitions.  When invoked when a prior declaration node corresponding to an implicit instantiation exists, the prior declaration node is repurposed to represent the explicit instantiation declaration or definition.  When no previous declaration node exists or when the previous node corresponds to an explicit declaration, a new node is allocated. Previously, in either case, the function attempted to propagate any existing `MSInheritanceAttr` attribute from the previous node, but did so regardless of whether the previous node was reused (in which case the repurposed previous node would gain a second attachment of the attribute; the second issue listed above) or a new node was created.  In the latter case, the attribute was not propagated before it was required to be present when compiling for C++17 or later (the first issue listed above).  The absent attribute resulted in an assertion failure that occurred during instantiation of the specialization definition when attempting to complete the definition in order to determine its alignment so as to resolve a lookup for a deallocation function for a virtual destructor.  This change addresses both issues by propagating the attribute closer in time to when a new `ClassTemplateSpecializationDecl` node is created and only when such a node is newly created.


Repository:
  rG LLVM Github Monorepo

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,25 @@
 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;
+class 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>;
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -10105,6 +10105,13 @@
         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 && PrevDecl->hasAttr<MSInheritanceAttr>()) {
+      Specialization->addAttr(PrevDecl->getAttr<MSInheritanceAttr>());
+      Consumer.AssignInheritanceModel(Specialization);
+    }
+
     if (!HasNoEffect && !PrevDecl) {
       // Insert the new specialization.
       ClassTemplate->AddSpecialization(Specialization, InsertPos);
@@ -10221,11 +10228,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.553551.patch
Type: text/x-patch
Size: 3166 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230825/8b69489b/attachment.bin>