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

[Generator] Generator does not create BlittablePInvokes when a boolean is present in a parameter #18685

Closed
mandel-macaque opened this issue Aug 9, 2023 · 1 comment · Fixed by #19034
Labels
generator Issues affecting the generator
Milestone

Comments

@mandel-macaque
Copy link
Member

When we did not need to worry about NativeAOT we could write a binding in the following way:

[Mac (14,0)]
[Export ("presentViewController:asPopoverRelativeToRect:ofView:preferredEdge:behavior:hasFullSizeContent:")]
void Present (NSViewController viewController, CGRect positioningRect, NSView positioningView, NSRectEdge preferredEdge, NSPopoverBehavior behavior, byte hasFullSizeContent);

Yet, now the above code will result in the following cecil test failure:

Cecil.Tests.BlittablePInvokes.CheckForNonBlittablePInvokes: Failures: In the file tests/cecil-tests/BlittablePInvokes.cs, read the guide carefully.
Expected: <empty>
But was: < ("ObjCRuntime.NativeHandle ObjCRuntime.Messaging::NativeHandle_objc_msgSend_nfloat_bool(System.IntPtr,System.IntPtr,System.Runtime.InteropServices.NFloat,System.Boolean)", <string.Empty>), ("ObjCRuntime.NativeHandle ObjCRuntime.Messaging::NativeHandle_objc_msgSendSuper_nfloat_bool(System.IntPtr,System.IntPtr,System.Runtime.InteropServices.NFloat,System.Boolean)", <string.Empty>), ("System.Void ObjCRuntime.Messaging::void_objc_msgSend_NativeHandle_CGRect_NativeHandle_UIntPtr_IntPtr_bool(System.IntPtr,System.IntPtr,ObjCRuntime.NativeHandle,CoreGraphics.CGRect,ObjCRuntime.NativeHandle,System.UIntPtr,System.IntPtr,System.Boolean)", <string.Empty>), ("System.Void ObjCRuntime.Messaging::void_objc_msgSendSuper_NativeHandle_CGRect_NativeHandle_UIntPtr_IntPtr_bool(System.IntPtr,System.IntPtr,ObjCRuntime.NativeHandle,CoreGraphics.CGRect,ObjCRuntime.NativeHandle,System.UIntPtr,System.IntPtr,System.Boolean)", <string.Empty>) **>**

You can see that one of the offending methods is:

ObjCRuntime.Messaging::void_objc_msgSend_NativeHandle_CGRect_NativeHandle_UIntPtr_IntPtr_bool(System.IntPtr,System.IntPtr,ObjCRuntime.NativeHandle,CoreGraphics.CGRect,ObjCRuntime.NativeHandle,System.UIntPtr,System.IntPtr,System.Boolean)"

A workaround to the issue is to change the binding to the following:

[Internal]
[Mac (14,0)]
[Export ("presentViewController:asPopoverRelativeToRect:ofView:preferredEdge:behavior:hasFullSizeContent:")]
void _Present (NSViewController viewController, CGRect positioningRect, NSView positioningView, NSRectEdge preferredEdge, NSPopoverBehavior behavior, byte hasFullSizeContent);

[Mac (14,0)]
[Wrap ("_Present (viewController, positioningRect, positioningView, preferredEdge, behavior, hasFullSizeContent ? (byte) 1 : (byte) 0)")]
void Present (NSViewController viewController, CGRect positioningRect, NSView positioningView, NSRectEdge preferredEdge, NSPopoverBehavior behavior, bool hasFullSizeContent);

NONE of or users expect to have to change all their bindings that take a boolean to take a byte and do a wrap with hasFullSizeContent ? (byte) 1 : (byte) 0.

Either the test is wrong or the generator needs to be able to deal with the boolean parameter without asking the user to do the conversion to a byte.

@rolfbjarne rolfbjarne added the generator Issues affecting the generator label Aug 10, 2023
@rolfbjarne
Copy link
Member

We have a rather long list of known failures:

static HashSet<string> knownFailuresPInvokes = new HashSet<string> {

Most of these are due to limitations in the generator that needs to be fixed, so just add the new signature there.

Keeping this bug open to fix the generator (and the corresponding known failures).

@rolfbjarne rolfbjarne added this to the .NET 8 milestone Aug 10, 2023
@rolfbjarne rolfbjarne added this to Optimizations in .NET 8 - Themes Aug 10, 2023
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 15, 2023
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 18, 2023
…marin#18685.

This means:

* Change all bool and char arguments in P/Invokes to be byte and ushort, respectively.
* Change all out/ref arguments to be pointers instead.
* Update managed binding code accordingly.
* Update a struct (GKTriangle) to not use a MarshalAs field, but instead only use blittable fields.
* Update tests accordingly.

Fixes xamarin#18685.
.NET 8 - Themes automation moved this from Optimizations to Done Sep 19, 2023
rolfbjarne added a commit that referenced this issue Sep 19, 2023
…8685. (#19034)

This means:

* Change all bool and char arguments in P/Invokes to be byte and ushort, respectively.
* Change all out/ref arguments to be pointers instead.
* Update managed binding code accordingly.
* Update a struct (GKTriangle) to not use a MarshalAs field, but instead only use blittable fields.
* Update tests accordingly.

One side effect is that legacy binding projects may need a reference to
the `System.Runtime.CompilerServices.Unsafe` NuGet now (this is a
built-in dependency in .NET) in order to compile successfully.

Fixes #18685.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Issues affecting the generator
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants