-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Change C# names of P/Invokes of objc_msgSend to be call-site specific. #47608
Conversation
Tagging subscribers to this area: @safern, @tannergooding Issue DetailsAs requested in #47592 (comment), I've renamed all instances of objc_msgSend P/Invokes to have call-site specific names since objc_msgSend has callsite-specific signatures.
|
[DllImport("libobjc.dylib", EntryPoint = "objc_msgSend_stret")] | ||
public static extern void get_bounds(ref Rect arect, IntPtr basePtr, IntPtr selector); | ||
[DllImport("libobjc.dylib", EntryPoint = "objc_msgSend")] | ||
public static extern bool get_isFlipped(IntPtr handle, IntPtr selector); |
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.
What is size of bool
in Objective-C - does it match what the marshaler is going to use here?
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.
It is not. This is a bug. I'll fix it tomorrow.
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.
Is the "gist" linked from https://docs.microsoft.com/en-us/xamarin/ios/internals/objective-c-selectors buggy too then?
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.
Yes, and it will cause failures on MacOS arm64.
Also change src\libraries\common\src\Interop\OSX\Interop.libobjc.cs for consistency? |
Already done 👍🏻 |
Ah, I have missed that it is part of this PR. |
We can’t really rename that one very well with the given model since we already have to customize the entry-point based on platform. |
[DllImport("libobjc.dylib", EntryPoint = "objc_msgSend")] | ||
public static extern IntPtr intptr_objc_msgSend(IntPtr basePtr, IntPtr selector); | ||
[DllImport("libobjc.dylib", EntryPoint = "objc_msgSend_stret")] | ||
public static extern void void_objc_msgSend_stret_rrect(ref Rect arect, IntPtr basePtr, IntPtr selector); |
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.
Would this look better as rect_objc_msgSend_stret(out Rect arect
? The Xamarin examples are not 100% consistent - majority of them are using out
, a few exception are using ref
for some reason.
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 don't think it particularly matters. We could also have the Rect type as the return value if we want.
Should the per-platform customization include change in the return value as well ( Is there an established pattern that the Xamarin bindings use to deal with this difference between platforms? |
We can do return everywhere and it will work. The Xamarin bindings (at least the ones on my VS4Mac instance) use out parameters for Which would you prefer that we do? |
I think we can
Either one of these is fine with me. |
I decided to follow the Xamarin convention. |
src/libraries/System.Drawing.Common/src/System/Drawing/macFunctions.cs
Outdated
Show resolved
Hide resolved
Hello @jkoritzinsky! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
As requested in #47592 (comment), I've renamed all instances of objc_msgSend P/Invokes to have call-site specific names since objc_msgSend has callsite-specific signatures.