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

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clang/test/CodeGenCXX/microsoft-abi-member-pointers.cpp:958
+extern template class a<int>;
+template class a<int>;
+}
----------------
tahonermann wrote:
> rnk wrote:
> > My expectation here is that we assign the unknown inheritance model to `a<int>`, is that right? Can you add a static_assert to that effect, or add some CHECK lines for the structure,  maybe make an alloca of type `void (a<int>::*var)()`  and check for the allocated type (it should be a struct with a pointer with lots of i32s)?
> Yes, when a pointer to member is formed for an incomplete class, in the absence of a `#pragma pointers_to_members` directive, use of inheritance keywords like `__single_inheritance`, or use of the `/vmb` or `/vmg` options (or equivalents), the "full_generality" / "virtual_inheritance" model is selected. I did verify that manually.
> 
> I can add a `static_assert` so long as it follows the explicit template instantiation definition. If it is placed earlier, then code related to obtaining a complete type is run and that ends up avoiding the assertion failure. See https://godbolt.org/z/qzTTfdfY1. This might indicate there is a bug elsewhere; I find it surprising that the early `static_assert` has that effect.
Got it, that makes sense. It's really just anything to document the requirement that we expect to get the most general member pointer representation from this code (notwithstanding global member pointer settings).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158869