xamarin-macios
xamarin-macios copied to clipboard
.NET: Use UnmanagedCallersOnlyAttribute instead of (or in addition to) MonoPInvokeCallbackAttribute
.NET 5/C# 9 has a new UnmanagedCallersOnlyAttribute, which we should start using instead of the MonoPInvokeCallbackAttribute (the main advantage is that the compiler will tell you if you use it wrong, as opposed to the MonoPInvokeCallbackAttribute, where the AOT compiler will crash and not tell you why if you use it wrong).
Ref: https://github.com/dotnet/runtime/issues/47177
Using UnmanagedCallersOnlyAttribute for block callbacks is rather complex, and has been moved to a separate issue: #15783 (see https://github.com/xamarin/xamarin-macios/issues/10470#issuecomment-933443703).
I'd rather have it instead of (than in addition to) so we're sure we're testing the new attribute and behaviour. It also reduce the amount of final metadata inside the app - even if that should not be a huge size :)
The old/existing attribute won't be in CoreCLR, which is pri0 https://github.com/xamarin/xamarin-macios/issues/10569
That makes it a pre-requirement to fix this earlier than pri1
The old/existing attribute won't be in CoreCLR, which is
pri0#10569 That makes it a pre-requirement to fix this earlier thanpri1
The MonoPInvokeCallbackAttribute is only required on platforms where we run the AOT compiler. CoreCLR won't be available (at least for now) on any platforms where we need the AOT compiler. Thus I don't think we need to implement this before CoreCLR support (and we define the attribute ourselves).
I don't recall if all those existing attributes are under #if !MONOMAC or similar conditionals...
If so then it's fine, if not then we have to exclude them (which is about the same work as replacing them)
It turns out this is a rather involved task, in particular when it comes to blocks:
- Add support to the BlockLiteral struct to accept function pointers. This is not trivial (might even be easier to create a new type instead of adding support to the existing type).
- Add support to the generator for the new block implementation.
- Add support to our optimizations to handle the new block implementation.
- Update all our manual binding code to use the new block implementation.
In the normal case (where the callback is supposed to be just a plain C function), things are a bit simpler, in that we can port our manual binding code incrementally.
I've ported a single callback (which at least makes sure that using function pointers work): https://github.com/xamarin/xamarin-macios/pull/12920
Since the work for blocks is not trivial, and requires some design and thought, it can be postponed until .NET 7.
Adding to keep track of incomplete work for using [UnmanagedCallersOnly]
The following files use SetupBlockUnsafe (this is not exhaustive)
AddressBook
CoreText
CoreGraphics/PDFArray.cs
The following packages use SetupBlock or SetupBlockUnsafe: AddressBook - SetupBlockUnsafe CoreText - SetupBlockUnsafe CoreGraphics - PDFArray.cs, SetupBlockUnsafe Metal Network ObjCRuntime UIKit VideoToolbox
ImageIO - mixed usage of callback as delegate and an unmanaged delegate. This is weird.
@stephen-hawley is this all done now?
@stephen-hawley is this all done now?
The generator needs to be updated too: https://github.com/xamarin/xamarin-macios/issues/18685
I believe this is complete now, we're only missing manual P/Invokes (neither the generator nor blocks), which is handled in #15684.