Page MenuHomePhabricator

Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

Authored by hans on Jul 31 2019, 6:52 AM.



This is similar to r245139, but that only addressed dllexported classes. It was still possible to run into the same problem with dllexported members in an otherwise normal class (see bug). This uses the same strategy to fix: delay defining the method until the whole class has been parsed.

(The easiest way to see the ordering problem is in Parser::ParseCXXMemberSpecification(): it calls ParseLexedMemberInitializers() *after* ActOnFinishCXXMemberDecls(), which was trying to define the dllexport method. Now we delay it to ActOnFinishCXXNonNestedClass() which is called after both of those.)

Diff Detail


Event Timeline

hans created this revision.Jul 31 2019, 6:52 AM
rnk added inline comments.Jul 31 2019, 9:54 AM
11545 ↗(On Diff #212568)

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
hans marked an inline comment as done.Jul 31 2019, 10:03 AM
hans added inline comments.
11545 ↗(On Diff #212568)

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.

rnk accepted this revision.Jul 31 2019, 10:48 AM


11545 ↗(On Diff #212568)

Right, I see. Nevermind then.

This revision is now accepted and ready to land.Jul 31 2019, 10:48 AM
aganea added inline comments.Jul 31 2019, 11:42 AM
11545 ↗(On Diff #212568)

This works on latest MSVC:

$ cat
struct Bar { Bar(); };
template <typename T> struct Foo { __declspec(dllexport) Foo() = default; Bar b; };
template class Foo<int>;

$ cl /c /Z7   <-- crashes if using clang-cl

$ lib test.obj /def

$ cat
struct Bar { Bar() { } };
template <typename T> struct Foo { __declspec(dllimport) Foo() = default; Bar b; };
int main() {
  Foo<int> f;
  return 0;

$ cl /c /Z7

$ link test.lib /DEBUG
This revision was automatically updated to reflect the committed changes.
hans marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2019, 1:00 AM
hans added a comment.Nov 5 2019, 7:15 AM

Sorry, seems I forgot to submit this comment..

11545 ↗(On Diff #212568)

Thanks! I believe that's a separate issue, though, and not related to this patch. I've filed for it.