Page MenuHomePhabricator

Delay emitting members of dllexport classes until the class is fully parsed (PR23542)
ClosedPublic

Authored by hans on Aug 7 2015, 3:48 PM.

Details

Summary

This enables Clang to correctly handle code such as:

struct __declspec(dllexport) S {
  int x = 42;
};

where it would otherwise error due to trying to generate the default constructor before the in-class initializer for x has been parsed.

Diff Detail

Repository
rL LLVM

Event Timeline

hans updated this revision to Diff 31551.Aug 7 2015, 3:48 PM
hans retitled this revision from to Delay emitting members of dllexport classes until the class is fully parsed (PR23542).
hans updated this object.
hans added a reviewer: rsmith.
hans added subscribers: cfe-commits, hansw.
hans updated this revision to Diff 31564.Aug 7 2015, 6:09 PM

Turns out ActOnFinishCXXMemberDefaultArgs needs to be re-entrant.

rsmith added inline comments.Aug 14 2015, 3:35 PM
lib/Sema/SemaDeclCXX.cpp
4700–4703 ↗(On Diff #31564)

Can we bail out of the function early in this case?

9501 ↗(On Diff #31564)

Can you rename this to something more general, like ActOnFinishCXXNonNestedClass, now that we're using it to check things other than default arguments?

hans updated this revision to Diff 32201.Aug 14 2015, 4:28 PM
hans updated this object.
hans marked an inline comment as done.

Addressed comments.

rsmith accepted this revision.Aug 14 2015, 4:45 PM
rsmith edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 14 2015, 4:45 PM
This revision was automatically updated to reflect the committed changes.

Cross-referencing PR40006. It seems to be a different manifestation of this same bug.

@hans Could you possibly take a look at the bug above whenever you have time? Many thanks!

hans added a comment.Dec 18 2018, 7:02 AM

Cross-referencing PR40006. It seems to be a different manifestation of this same bug.

@hans Could you possibly take a look at the bug above whenever you have time? Many thanks!

Sorry, I saw the bug but didn't have time to look at it yet. I will try to get to it tomorrow.

lib/Sema/SemaDeclCXX.cpp
4700–4703 ↗(On Diff #31564)

Yes. Done.

9514 ↗(On Diff #31564)

I was trying to fancy and use C++11 here, but I think I really just want good-old std::swap, because IIUC, I'm not supposed to use a var after std::move'ing from it?