Skip to content

Conversation

ahmedshakill
Copy link
Contributor

Implements #1812

Copy link

github-actions bot commented Aug 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@tommymcm tommymcm left a comment

Choose a reason for hiding this comment

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

lgtm, some nits

@ahmedshakill ahmedshakill requested a review from tommymcm August 25, 2025 06:16
Copy link
Collaborator

@tommymcm tommymcm left a comment

Choose a reason for hiding this comment

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

CIRGen lgtm
Some more changes for the test to format it correctly and make it more readable.

Copy link
Collaborator

@tommymcm tommymcm left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM after minor comment


QualType thisTy =
IsArrow ? Base->getType()->getPointeeType() : Base->getType();
emitCXXDestructorCall(globalDecl, callee, This.getPointer(), thisTy,
Copy link
Member

Choose a reason for hiding this comment

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

I see some code repetition from the else part, that can be improved overall

Copy link
Collaborator

Choose a reason for hiding this comment

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

Classic codegen, calls CGM.getCXXABI().EmitVirtualDestructorCall() here. Is there a reason we can't do that?

// CIR-NEXT: %[[DTOR:.*]] = cir.load align(8) %[[DTOR_PTR]] : !cir.ptr<!cir.ptr<!cir.func<(!cir.ptr<!rec_A>)>>>, !cir.ptr<!cir.func<(!cir.ptr<!rec_A>)>>
// CIR-NEXT: cir.call %[[DTOR]](%[[THIS]]) : (!cir.ptr<!cir.func<(!cir.ptr<!rec_A>)>>, !cir.ptr<!rec_A>) -> ()
// CIR-NEXT: cir.trap
// CIR-NEXT: }
Copy link
Member

Choose a reason for hiding this comment

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

Add LLVM and OGCG tests!

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.

4 participants