-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[SILGen] Fix a bug where the wrong convention was being used for lowering a closure to a C++ function pointer #73039
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
Conversation
lowering a closure to a C++ function pointer Use the C function pointer convention instead of the block convention. rdar://122977380
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Akira!
I have just a couple of comments regarding tests.
@@ -141,3 +141,8 @@ module CopyMoveAssignment { | |||
header "copy-move-assignment.h" | |||
requires cplusplus | |||
} | |||
|
|||
module NonTrivialClasses { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call this module Closures
, since closures are the special thing in this test module?
@@ -0,0 +1,21 @@ | |||
// RUN: %target-swiftxx-frontend -I %S/Inputs -emit-silgen %s | %FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please also add an execution test? That would help to make sure we don't crash in IRGen or at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add an execution test, but IRGen will still be incorrect until we fix #73019.
Notice that the parameter of the thunk in the test case is @in NonTrivial
, which means the destructor is called in the thunk. Calling cfunc2
would result in a double-free crash if NonTrivial
was defined as follows:
struct NonTrivial {
NonTrivial() { p = new int(123); }
~NonTrivial() { delete p; }
int *p = nullptr;
};
void cfunc2(void (*fp)(NonTrivial)) {
NonTrivial t;
(*fp)(t);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thank you!
- Rename module. - Fix check string in test - Add execution test
@swift-ci please test |
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@swift-ci please test |
Use the C function pointer convention instead of the block convention.
rdar://122977380