[PATCH] D65511: Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Sorry, seems I forgot to submit this comment..



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11545
+    for (CXXMethodDecl *M : WorkList) {
+      DefineImplicitSpecialMember(*this, M, M->getLocation());
+      ActOnFinishInlineFunctionDef(M);
----------------
aganea wrote:
> rnk wrote:
> > hans wrote:
> > > rnk wrote:
> > > > Do we need to do this until fixpoint? Suppose a dllexported implicit special member triggers a template instantiation, and the template has a dllexported defaulted special member, something like:
> > > > ```
> > > > struct Bar { Bar(); };
> > > > template <typename T> struct Foo { __declspec(dllexport) Foo() = default; Bar obj; };
> > > > struct Baz {
> > > >    ... not sure how to trigger instantiation
> > > > };
> > > > ```
> > > I think that should work, and that's why the function is written to be re-entrant by having a local worklist. If it triggers a template instantiation, ActOnFinishCXXNonNestedClass should get called for the newly instantiated class. But I'm also not sure how to write a test case that would trigger it.
> > Right, I see. Nevermind then.
> This works on latest MSVC:
> ```
> $ cat test.cc
> struct Bar { Bar(); };
> template <typename T> struct Foo { __declspec(dllexport) Foo() = default; Bar b; };
> template class Foo<int>;
> 
> $ cl /c /Z7 test.cc   <-- crashes if using clang-cl
> 
> $ lib test.obj /def
> 
> $ cat test2.cc
> struct Bar { Bar() { } };
> template <typename T> struct Foo { __declspec(dllimport) Foo() = default; Bar b; };
> int main() {
>   Foo<int> f;
>   return 0;
> }
> 
> $ cl /c /Z7 test2.cc
> 
> $ link test.lib test2.cc /DEBUG
> ```
Thanks! I believe that's a separate issue, though, and not related to this patch. I've filed https://bugs.llvm.org/show_bug.cgi?id=42857 for it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65511