This is an archive of the discontinued LLVM Phabricator instance.

MS ABI: Implement copy-ctor closures, finish implementing throw
ClosedPublic

Authored by majnemer on Mar 10 2015, 1:08 PM.

Details

Summary

This adds support for copy-constructor closures. These are generated
when the C++ runtime has to call a copy-constructor with a particular
calling convention or with default arguments substituted in to the call.

Because the runtime has no mechanism to call the function with a
different calling convention or know-how to evaluate the default
arguments at run-time, we create a thunk which will do all the
appropriate work and package it in a way the runtime can use.

Diff Detail

Repository
rL LLVM

Event Timeline

majnemer updated this revision to Diff 21621.Mar 10 2015, 1:08 PM
majnemer retitled this revision from to MS ABI: Implement copy-ctor closures, finish implementing throw.
majnemer updated this object.
majnemer added a reviewer: rnk.
majnemer added a subscriber: Unknown Object (MLST).
rnk added inline comments.Mar 10 2015, 2:26 PM
lib/AST/MicrosoftMangle.cpp
790 ↗(On Diff #21621)

Keep the Ctor_Complete assertion to avoid decl mangling collisions?

lib/CodeGen/MicrosoftCXXABI.cpp
3332–3335 ↗(On Diff #21621)

Surely this can be:

CD->getType()->getAs<FunctionType>()->getCallConv()?

Does the canonical unqualified type step matter in practice?

3336 ↗(On Diff #21621)

Can you test the calling convention difference case and the variadic case?

lib/Sema/SemaExprCXX.cpp
829 ↗(On Diff #21621)

I can see that you need to call BuildCXXDefautlArgExpr to do all the necessary semantic checking, but is it really necessary to maintain a map of all the CXXDefaultArgExprs, rather than simply calling ParmVarDecl::getDefaultArgExpr() during CodeGen?

majnemer added inline comments.Mar 10 2015, 3:38 PM
lib/AST/MicrosoftMangle.cpp
790 ↗(On Diff #21621)

That assertion was never like because Structor was always null for constructors. This assertion is actually wrong in practice because Ctor_Base can happen and weakening the assertion doesn't seem very useful.

lib/CodeGen/MicrosoftCXXABI.cpp
3332–3335 ↗(On Diff #21621)

Done.

3336 ↗(On Diff #21621)

The different calling convention case is the variadic case.

A test for the variadic case has been added.

lib/Sema/SemaExprCXX.cpp
829 ↗(On Diff #21621)

Yes because the we won't have an instantiated default argument at CodeGen-time, just the dependent one which won't be suitable.

majnemer updated this revision to Diff 21665.Mar 10 2015, 5:24 PM
  • Addressed review comments.
rnk added inline comments.Mar 10 2015, 5:27 PM
include/clang/Basic/ABI.h
28–29 ↗(On Diff #21665)

I'm not super psyched about adding to this enum. What do you think about adding custom entry points to the MS C++ ABI that just tweak internal bools?

lib/AST/MicrosoftMangle.cpp
782–788 ↗(On Diff #21665)

Oh dear, does isCopyConstructor() return false for the variadic test case?

rnk accepted this revision.Mar 11 2015, 11:06 AM
rnk edited edge metadata.

lgtm

include/clang/Basic/ABI.h
28–29 ↗(On Diff #21665)

Sure, if we think of this as just an enum of different constructor manglings, then I guess this is OK.

test/CodeGenCXX/microsoft-abi-throw.cpp
78 ↗(On Diff #21665)

Maybe s/Bad/DoubleDefault/?

This revision is now accepted and ready to land.Mar 11 2015, 11:06 AM
This revision was automatically updated to reflect the committed changes.