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

NSCoding in iOS binding causes linker-cache compile errors #21256

Open
LeadAssimilator opened this issue Sep 17, 2024 · 6 comments
Open

NSCoding in iOS binding causes linker-cache compile errors #21256

LeadAssimilator opened this issue Sep 17, 2024 · 6 comments
Labels
bug If an issue is a bug or a pull request a bug fix
Milestone

Comments

@LeadAssimilator
Copy link

LeadAssimilator commented Sep 17, 2024

Apple platform

iOS

Framework version

net8.0-*

Affected platform version

Microsoft.iOS.Sdk.net8.0_17.5/17.5.8030

Description

Given the following native code:

typedef NSDictionary<NSString *, NSObject<NSCoding> *> GCKSessionOptions;
@property(nonatomic, strong, readonly, nullable) GCKSessionOptions *sessionOptions;

And corresponding sharpie binding:

// @property (readonly, nonatomic, strong) GCKSessionOptions * _Nullable sessionOptions;
[NullAllowed, Export ("sessionOptions", ArgumentSemantic.Strong)]
NSDictionary<NSString, NSCoding> SessionOptions { get; }

Yields the following compiler error:

/usr/local/share/dotnet/packs/Microsoft.iOS.Sdk.net8.0_17.5/17.5.8030/targets/Xamarin.Shared.Sdk.targets(1459,3): error : /proj/obj/Release/net8.0-ios/ios-arm64/linker-cache/registrar.h:2983:33: error: no type or protocol named 'Microsoft_iOS__Foundation_NSCoding' [/proj/proj.csproj::TargetFramework=net8.0-ios]
/usr/local/share/dotnet/packs/Microsoft.iOS.Sdk.net8.0_17.5/17.5.8030/targets/Xamarin.Shared.Sdk.targets(1459,3): error :         -(NSDictionary <NSString *, id<Microsoft_iOS__Foundation_NSCoding>>*) sessionOptions; [/proj/proj.csproj::TargetFramework=net8.0-ios]
/usr/local/share/dotnet/packs/Microsoft.iOS.Sdk.net8.0_17.5/17.5.8030/targets/Xamarin.Shared.Sdk.targets(1459,3): error :                                        ^ [/proj/proj.csproj::TargetFramework=net8.0-ios]
/usr/local/share/dotnet/packs/Microsoft.iOS.Sdk.net8.0_17.5/17.5.8030/targets/Xamarin.Shared.Sdk.targets(1459,3): error : /proj/obj/Release/net8.0-ios/ios-arm64/linker-cache/registrar.h:2988:114: error: no type or protocol named 'Microsoft_i [/proj/proj.csproj::TargetFramework=net8.0-ios]

Steps to Reproduce

  1. Create a new maui ios binding project for GoogleCastSDK 4.8.3 (or native library with similar sessionOptions property in a NSObject).
  2. Add the framework and generate sharpie bindings w/ appropriate fixups
  3. Create a new default maui application
  4. Add a project reference to binding project
  5. Add a code reference to offending class from the binding project
  6. Build for simulator and observe success
  7. Build for device and observe compiler error

Did you find any workaround?

Use the arguably more correct INSCoding for the binding instead of NSCoding.

Build logs

No response

@LeadAssimilator
Copy link
Author

While the proper binding in this case is arguably INSCoding instead of NSCoding or even NSObject (both cause the registrar to use id), something doesn't appear to be correct in the registrar code gen when using NSCoding. I think this used to work in old versions ok, and the fact this only manifests in release builds (at least in the default configuration) makes these bugs more difficult to catch.

rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Sep 18, 2024
@rolfbjarne
Copy link
Member

I can reproduce the problem, and you're correct, the registrar is generating invalid code (test case shows this in rolfbjarne@48cd49a).

In your case the correct type to use is INSCoding instead of NSCoding (in fact NSCoding shouldn't even exist, so I've created a PR to eventually remove it (#21257)).

this only manifests in release builds

It manifests in device builds (even debug ones).

For performance reasons there are two rather different registrar implementations between device and simulator builds, and in this particular case it turns out the device version has a bug.

@rolfbjarne rolfbjarne added the bug If an issue is a bug or a pull request a bug fix label Sep 18, 2024
@rolfbjarne rolfbjarne added this to the Future milestone Sep 18, 2024
@LeadAssimilator
Copy link
Author

My bad on the debug vs. release - it slipped my mind that I also flipped to device as a force of habit. Glad that you reproduced it easily.

Once the models for various classes are removed, will sharpie pick that up or will it also need changes?

I do wonder why sharpie seems to prefer models over ifaces in the cases where either would suffice, since I think the iface is what you really want anyway, at least in all the cases I have encountered. Having it make the seemingly wrong choice just adds to the binding fixup tedium, along with having to still manually add iface skeletons and switch types to them in various cases, address all the verify attrs and fixup all the delegate method names it ruins. I realize it can't be perfect, but I do wish whomever is maintaining sharpie would actually use it on large and/or widely used libraries so it would improve.

@rolfbjarne
Copy link
Member

Once the models for various classes are removed, will sharpie pick that up or will it also need changes?

No, sharpie will keep doing what it's doing until we fix it - in this case the generated code just wouldn't compile because those types would be missing.

I realize it can't be perfect, but I do wish whomever is maintaining sharpie would actually use it on large and/or widely used libraries so it would improve.

Agreed, although as usual it comes down to resource management and never having enough time to do everything we want to do.

@LeadAssimilator
Copy link
Author

Not sure if this is related, but another registrar compile issue occurred on maccatalyst (didn't try others) with --registrar:static if using NSUrlSessionDataDelegate - INSUrlSessionDataDelegate works as expected though:

// -(instancetype _Nonnull)initWithDelegate:(id<NSURLSessionDataDelegate>  _Nonnull __strong)delegate delegateQueue:(NSOperationQueue * _Nonnull __strong)delegateQueue __attribute__((ns_consumes_self)) __attribute__((ns_returns_retained));
[Export ("initWithDelegate:delegateQueue:")]
NativeHandle Constructor (NSUrlSessionDataDelegate @delegate, NSOperationQueue delegateQueue);
obj/Debug/net8.0-maccatalyst/maccatalyst-x64/linker-cache/registrar.h:3805:29: error: type argument 'Microsoft_MacCatalyst__Foundation_NSUrlSessionDataDelegate' must be a pointer (requires a '*')
   3805 |         -(id) initWithDelegate:(id<Microsoft_MacCatalyst__Foundation_NSUrlSessionDataDelegate>)p0 delegateQueue:(NSOperationQueue *)p1;

@rolfbjarne
Copy link
Member

That's the exact same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug If an issue is a bug or a pull request a bug fix
Projects
None yet
Development

No branches or pull requests

2 participants