Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector deleting destructors #3

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

Fznamznon
Copy link
Owner

No description provided.

@Fznamznon
Copy link
Owner Author

TODO:

  • study commit ae9b070 to understand why MicrosoftCXXABI::setCXXDestructorDLLStorage does what it does with deleting destructors

if (RD->hasDefinition() && !RD->hasTrivialDestructor()) {
// FIXME: check devirtualization?
const CXXDestructorDecl *Dtor = RD->getDestructor();
RequiresVectorDestructor = Dtor->isVirtual();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely wrong. 3 is passed even when the destructor is not virtual at all.

Comment on lines +1657 to +1662
// llvm::Value *LoadThisForDtorDelete(CodeGenFunction &CGF,
// const CXXDestructorDecl *DD) {
// if (Expr *ThisArg = DD->getOperatorDeleteThisArg())
// return CGF.EmitScalarExpr(ThisArg);
// return CGF.LoadCXXThis();
// }
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// llvm::Value *LoadThisForDtorDelete(CodeGenFunction &CGF,
// const CXXDestructorDecl *DD) {
// if (Expr *ThisArg = DD->getOperatorDeleteThisArg())
// return CGF.EmitScalarExpr(ThisArg);
// return CGF.LoadCXXThis();
// }

Comment on lines 3897 to 3899
// FIXME: Actually, the dtor thunk should be emitted for vector deleting
// dtors rather than scalar deleting dtors. Just use the vector deleting dtor
// mangling manually until we support both deleting dtor types.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See into that fixme

if (Context.getTargetInfo().getCXXABI().isMicrosoft())
Out << "() [vector deleting]";
else
Out << "() [deleting]";
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Out << "() [deleting]";
Out << "() [scalar deleting]";


void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF, GlobalDecl GD,
CallArgList &CallArgs) override {
assert(GD.getDtorType() == Dtor_Deleting &&
"Only deleting destructor thunks are available in this ABI");
// assert(GD.getDtorType() == Dtor_Deleting &&
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be a scalar deleting dtor?

if (isa<CXXDestructorDecl>(GD.getDecl()))
assert(GD.getDtorType() == Dtor_Deleting);
if (isa<CXXDestructorDecl>(GD.getDecl())) {
assert(GD.getDtorType() == Dtor_Deleting ||
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(GD.getDtorType() == Dtor_Deleting ||
assert(

Sometimes new[] expression ends up in a function that is itself deferred
so we first emit scalar destructor as a deferred decl then meet new[]
for the type. Need to switch to vector deleting dtor then.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants