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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:10112
+      Specialization->addAttr(PrevDecl->getAttr<MSInheritanceAttr>());
+      Consumer.AssignInheritanceModel(Specialization);
+    }
----------------
erichkeane wrote:
> tahonermann wrote:
> > I am concerned that moving the call to `Consumer.AssignInheritanceModel()` to immediately after the creation of the node, but before it is populated might be problematic. Previously, this call was still made before the node was completely constructed (e.g., before `setTemplateSpecializationKind()` is called for it). It is difficult to tell if any consumers might be dependent on more of the definition being present. 
> SO I think we still need to do this off of the definition, right?  Else if `PrevDecl` ends up being a declaration (followed later by a definition that has the attribute), we'll end up missing it?  Typically attributes are 'added' by later decls, so only the 'latest one' (though attributes can't be added after definition IIRC) has 'them all'.
This handles the situation where a new node is created for either an explicit template instantiation declaration or definition; that matches prior behavior. The goal is to ensure each node, regardless of the reason for its creation, inherits the attribute from the previous declaration; that ensures that any explicitly declared attributes are checked for consistency (see `Sema::mergeMSInheritanceAttr()`).

Note that an explicit class template specialization is not allowed to follow an explicit template instantiation declaration or definition. https://godbolt.org/z/cbvaac717.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158869/new/

https://reviews.llvm.org/D158869