Add support for function pointers to BlockLiteral, and use it to update
almost all manually bound block code to use function pointers (in .NET).
Also add support to the linker for optimizing the new block API.
Contributes towards https://github.com/xamarin/xamarin-macios/issues/15783.
This is mostly converting 'bool' arguments to 'byte' arguments, and 'string'
arguments to 'IntPtr' with custom utf8->string conversions.
This is necessary in order to convert all block callbacks to use
UnmanagedCallersOnly function pointers (which can't have non-blittable types
in their signature).
Contributes towards https://github.com/xamarin/xamarin-macios/issues/15783.
* Make BlockLiteral disposable.
This also means to modify the cleanup logic so that it's safe to call
Dispose more than once.
* Create a helper class to create a block for a simple Action delegate.
This allows us to simplify a good chunk of code.
* Update all block creation to use the new block API, where blocks are
disposable. This makes the code pattern a lot simpler.
I've changed all the P/Invokes to use an unsafe 'BlockLiteral*' pointer,
because 'using' variables can't be passed as ref arguments, so the choice
was either to make the parameter type 'IntPtr' and cast away the pointer:
using var block = new BlockLiteral ();
PInvoke ((IntPtr) &block);
or make the parameter an unsafe 'BlockLiteral*' pointer:
unsafe {
using var block = new BlockLiteral ();
PInvoke (&block);
}
The upcoming support for function pointers don't have this choice:
function pointers are always unsafe, so I chose to go the unsafe route
here as well, since it makes the code simpler once support for function
pointers has been implemented.
Contributes towards https://github.com/xamarin/xamarin-macios/issues/15783.
This PR might be easier to review commit-by-commit.
Stop implying Mac Catalyst attributes from the iOS attributes, and instead
treat Mac Catalyst like all the other platforms (not implying anything from
any other platform).
This makes our attribute logic much easier to reason about and understand.
It also required adding numerous Mac Catalyst attributes that were previously
implied. This task was way too big to do manually, so I made some changes to
Chris' mellite tool, and managed to do it quite easily with Roslyn with the
changes in this branch: https://github.com/rolfbjarne/mellite/tree/explicit-maccatalyst-attributes
* If there's both an UnsupportedOSPlatform and ObsoletedOSPlatform attribute with
the same version, then remove the UnsupportedOSPlatform attribute. This is because
in the past we used [UnsupportedOSPlatform] + [Obsolete] to indicate that an API
is obsolete, but then the [ObsoletedOSPlatform] attribute was added, and we replaced
the [Obsolete] attributes with [ObsoletedOSPlatform] attributes, which makes the
[UnsupportedOSPlatform] attributes redundant/incorrect.
* If there's [UnsupportedOSPlatform] with a version or [ObsoletedOSPlatform] with
a version, then also add [SupportedOSPlatform] in a few cases.
* If there's an [UnsupportedOSPlatform] with a version for API that's obsolete/non-working,
then remove the version.
This PR teaches our code generator to generate .NET 6 style availability attributes, adds a 4th Cecil test to verify our generated attributes, and a metric ton of API changes to satisfy that test.
The generator work is the core of this PR, and includes:
- Hacking out chunks of generator.cs that "helpfully" remove duplicate attributes, which are no longer duplicate in the new order that NET6 attributes force upon us. See changes in FilterMinimumVersion and PrintPlatformAttributes
- Prevent a crash when the generator processes availability attributes with no version included (example: introduced on iOS but no version). See Is64BitiOSOnly.
- The meat, GetPlatformAttributesToPrint, which synthesizes many attributes "out of thing air" from:
- The parent context
- Implied introduced just because the class exists on a given framework at all
- Implied Catalyst because iOS exists
- A few cludgy hacks PrintPlatformAttributesNoDuplicates and GenerateProperty because the existing PrintPlatformAttributes did not pass down parent context down, and the refactor was dangerous/too time consuming given time pressure.
There are two intended API changes introduced by the reviews in this PR:
- GetCurrentInputDevice was obviously intended by availability attributes to exist on Catalyst but due to define confusion was excluded. It is an addition in Microsoft.MacCatalyst.dll only.
- The NEAppRule constructors were mis-marked on platforms, and were showing up incorrectly on Mac/Catalyst. I corrected the Catalyst one unconditionally, as we have not shipped Catalyst yet, but Mac is only fixed in NET6.
There is a lot of follow up work in https://github.com/xamarin/xamarin-macios/issues/14802 to do to remove a number of hard coded test failures, but this should be almost all of the remaining work in NET6 attributes.
🤞 this doesn't break too much for future us.
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
* [NET Attribute Conversion] Chip Framework
* [NET Attribute Conversion] Rerun with many fixes
* Fix generator crash when compiling attributes with no introduced version
* Test changes for availability re-run. One new test
* [Foundation] Make numerous CFArray and NSArray APIs take/return NativeHandle instead of IntPtr.
* [src] Fix a lot of other cases of IntPtr -> NativeHandle.
This is just fallout from the CFArray/NSArray in the previous commit.
* Remove the internal (IntPtr, bool) constructor, it has non-standard semantics and it's confusing.
* Enable nullability and fix code accordingly.
* Use 'is' and 'is not' instead of '==' and '!=' for object identity.
* Use 'nameof (parameter)' instead of string constants.
* [ObjCRuntime] Add a non-deprecated internal system-version checking API and use it everywhere.
The PlatformHelper class is deprecated, so implement a new version that isn't
deprecated, and which shares a similar API between all platforms - the Check*
methods includes the name of the platform, because that makes it clearer which
version we're talking about from the call site. There's a quirk though:
there's no separate ChecktvOS or CheckMacCatalyst, because the system version
is the same as for iOS, so we can just use 'iOS'.
For macOS we can now use NSProcessInfo.ProcessInfo.OperatingSystemVersion to
determine the OS version, because it's supported in all versions of macOS we
support for .NET.
Fixes issues such as this when building with XAMCORE_4_0:
> CoreMedia/CMSync.cs(590,11): error CS0103: The name 'PlatformHelper' does not exist in the current context
* Bring back PlatformHelper.CheckSystemVersion, but only for !NET.
* [tests] Remove 32-bit macOS logic, it's long dead.
* [introspection] Implement OS version check using 'NSProcessInfo.ProcessInfo.IsOperatingSystemAtLeastVersion' for macOS.
* [monotouch-test] Use TestRuntime.[Check|Assert]XcodeVersion instead of PlatformHelper.CheckSystemVersion.
* Subclass NativeObject to reuse object lifetime code.
* Enable nullability and fix code accordingly.
* Use 'is' and 'is not' instead of '==' and '!=' for object identity.
* Use CFString.CreateNative/ReleaseNative instead of other means to create
native strings (the fastest and least memory hungry option).
* Use the null-safe NativeObjectExtensions.GetHandle extension method to get
the handle instead of checking for null (avoids some code duplication).
* Use 'nameof (parameter)' instead of string constants.
* Enable nullability and fix code accordingly.
* Use 'is' and 'is not' instead of '==' and '!=' for object identity.
* Use CFString.CreateNative/ReleaseNative instead of other means to create
native strings (the fastest and least memory hungry option).
* Use the null-safe NativeObjectExtensions.GetHandle extension method to get
the handle instead of checking for null (avoids some code duplication).
* Use 'nameof (parameter)' instead of string constants.
* Subclass NativeObject to reuse object lifetime code.
* Enable nullability and fix code accordingly.
* Use 'is' and 'is not' instead of '==' and '!=' for object identity.
* Use the null-safe NativeObjectExtensions.GetHandle extension method to get
the handle instead of checking for null (avoids some code duplication).
* Use 'nameof (parameter)' instead of string constants.
* Use Array.Empty<T> instead of creating an empty array manually.
* Subclass NativeObject to reuse object lifetime code.
* Enable nullability and fix code accordingly.
* Use 'is' and 'is not' instead of '==' and '!=' for object identity.
* Use CFString.CreateNative/ReleaseNative instead of other means to create
native strings (the fastest and least memory hungry option).
* Use the null-safe NativeObjectExtensions.GetHandle extension method to get
the handle instead of checking for null (avoids some code duplication).
* Use 'nameof (parameter)' instead of string constants.
* Call 'GetCheckedHandle ()' (which will throw an ObjectDisposedException if
Handle == IntPtr.Zero) instead of manually checking for IntPtr.Zero and
throwing ObjectDisposedException.
* Simplify block creation code a bit.
* [cecil-tests] Add test for MarshalAs attributes for bool return type / parameters in P/Invokes.
This also meant adding support for resolving additional BCL assemblies (to be able to process Action<> and Func<> types).
* [generator] Add [MarshalAs (UnmanagedType.I1)] to bool return types and parameters.
* Fix all manually bound API to use a MarshalAs attribute for boolean return types and parameters in P/Invokes.
The generator has been able to read and re-generate enums for a while.
However not all enums were in file processed by the generator. That was
not a huge problem since the output was identical, unless this was a
"smart enum".
Supporting .net5+ `[Uns|S]upportedOSPlatformAttribute` on API requires
a lot of changes. Generated bindings are much easier since the generator
will output the new attributes for dotnet and keep the older ones for
_legacy_.
So moving the enums to generated code means a lot less code to update
for dotnet - and means additional enums (new Xcode) won't have to include
both new and legacy attributes.
The number of manual updates (for iOS) is about 1500 less (5792 down to
4219) than before.
Helps https://github.com/spouliot/xamarin-macios/pull/new/gh10170
The latter requires the spellchecker which varies by OS versions, so we
only run it on the latest OS version (and simulator, except macOS).
The former are some internal rules and can be run on every commit and
avoid finding issues late in a release cycle.
Also changed to
- process members even if a type is obsoleted
- process the properties and events on types
About the strings...
Some were fine, others were not. Copy/pasted strings are hard to
maintain. Moving them to constants will help, both maintainability and
will reduce the metadata size of the platform assemblies.
ref: https://github.com/xamarin/xamarin-macios/issues/9353
Xcode 11 doesn't support anything below iOS 7.0 (the linker will automatically
change the deployment target to 7.0), so we need to drop support as well
(since our native bits will be targetting iOS 7.0, and we can't change that).
https://github.com/xamarin/xamarin-macios/issues/6213
A change in the linker made a potential problem show up as a registrar error
instead of a runtime exception.
The linker became smarter in d16-2, and will now remove interface
implementations in certain cases. In particular, the linker will remove
interface implementations for a type that is never instantiated (which means
there will never be any instances of that type, which means the interface
won't be needed).
The ABRecord was abstract, and as such there will never be any ABRecord
instances. If none of ABRecord's subclasses are used in the app, there won't
be instances of ABRecord subclasses either.
This meant the linker removed all of ABRecord's interfaces, including
INativeObject.
We have API that uses ABRecord in the signature. The registrar needs to know
if a particular type is an INativeObject, which is determined by checking if
the type implements the INativeObject interface. Since the INativeObject
interface implementation was removed, the registrar determined that the
ABRecord type in the signature is invalid, and showed an error:
Error MT4136: The registrar cannot marshal the parameter type 'AddressBook.ABRecord' of the parameter 'address' in the method 'PassKit.PKPaymentAuthorizationViewController/_PKPaymentAuthorizationViewControllerDelegate.DidSelectShippingAddress(PassKit.PKPaymentAuthorizationViewController,AddressBook.ABRecord,PassKit.PKPaymentShippingAddressSelected)
The actual problem is not the linker change, but the fact that a signature
uses an abstract type. At runtime, this will cause problems if we ever need to
instantiate such a managed type: we can't. This is generally not a problem for
NSObject subclasses, because we can determine the runtime type of the object,
and that's usually [1] some other, non-abstract type. However, for
INativeObjects, we can't find a better type at runtime (because we can't
determine the runtime type of the object), so we're left with trying to
instantiate the exact type in the signature. If this is an abstract type,
things will go badly.
So make ABRecord non-abstract.
Fixes https://github.com/xamarin/xamarin-macios/issues/6654.
[1] Usually, but not always, as history has shown. See also https://github.com/xamarin/xamarin-macios/issues/4969.
* [linker] Optimize calls to BlockLiteral.SetupBlock to inject the block signature.
Optimize calls to BlockLiteral.SetupBlock[Unsafe] to calculate the block
signature at build time, and inject it into the call site.
This makes block invocations 10-15x faster (I've added tests that asserts at
least an 8x increase).
It's also required in order to be able to remove the dynamic registrar code in
the future (since calculating the block signature at runtime requires the
dynamic registrar).
* [mtouch/mmp] Add support for reporting errors/warnings that point to the code line causing the error/warning.
Add support for reporting errors/warnings that point to the code line causing
the error/warning by adding ErrorHelper overloads that take the exact
instruction to report (previously we defaulted to the first line/instruction
in a method).
* [tests] Add support for asserting filename/linenumber in warning messages.
* Make all methods that manually create BlockLiterals optimizable.
* [tests] Create a BaseOptimizeGeneratedCodeTest test that's included in both XI's and XM's link all test.
* [tests] Add link all test (for both XI and XM) to test the BlockLiteral.SetupBlock optimization.
* [tests] Add mtouch/mmp tests for the BlockLiteral.SetupBlock optimization.
* [tests][linker] Make the base test class abstract, so tests in the base class aren't executed twice.
* [tests][linker] Don't execute linkall-only tests in linksdk.
The optimization tests only apply when the test assembly is linked, and that
only happens in linkall, so exclude those tests in linksdk.
* [tests][mmptest] Update test according to mmp changes.
Fixes these test failures:
1) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("inline-runtime-arch")
The warning 'MM0132: Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
Message #1 did not match:
actual: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
Message #2 did not match:
actual: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'inline-runtime-arch'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
2) Failed : Xamarin.MMP.Tests.MMPTests.MM0132("foo")
The warning 'MM0132: Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.' was not found in the output:
Message #1 did not match:
actual: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
Message #2 did not match:
actual: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock.'
expected: 'Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size.'
* [tests][linker] Fix typo.
Fixes this test failure:
1) SetupBlock_CustomDelegate (Linker.Shared.BaseOptimizeGeneratedCodeTest.SetupBlock_CustomDelegate)
Counter
Expected: 1
But was: 2
* [registrar] Minor adjustment to error message to match previous (and better) behavior.
Fixes this test failure:
1) Failed : Xamarin.Registrar.GenericType_WithInvalidParameterTypes
The error 'MT4136: The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)'' was not found in the output:
Message #1 did not match:
actual: 'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<Foundation.NSObject>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''
expected: 'The registrar cannot marshal the parameter type 'System.Collections.Generic.List`1<U>' of the parameter 'arg' in the method 'Open`1.Bar(System.Collections.Generic.List`1<U>)''
* [docs] mmp shows MM errors/warnings.
* [docs] Improve according to reviews.
* [tests] Fix merge failure causing test duplication.
You were the preprocessor we wished C# had natively
Removing PMCS requires these changes:
* Remove XamCore from src/
* Remove XamCore from tools/
* Remove XamCore from runtime/
* nint/nuint enum conversion
* _compat_ enum conversion
* NSAction conversion
* Hand fix single API incorrectly converted by PMCS to unbreak compatibility
- Due to a bug in PMCS, the nuint was incorrectly converted in this API.
- However, as that ship as sailed, we must "fix" it until XAMCORE_4_0
* Update readme
* Bump macios-binaries
We normally frown on large scale _cosmetic_ changes, mostly because it breaks git's history (very useful) and makes merging branches harder and more error prone (very annoying).
However we require, right now, such changes to remove our old, mcs-based, pre-processor (pmcs) so it's a _good_ time to address the old, unneeded availability attributes - since most of them are re-written for our next milestone.
This won't change the final application size in most cases, as the linker removes them, but it will make the (unlinked) platform assemblies smaller. This means they will load faster (e.g. by mtouch, mmp, IDE, workbooks...) and will reduce the time/memory needed to reflect them.
* Change BlockLiteral.SetupBlock to keep a reference to the delegate passed as the trampoline.
Change BlockLiteral.SetupBlock to keep a reference to the delegate passed as
the trampoline, so that it's safer for normal users (crashes due to incorrect
usage are rare and random, and as such they're also hard to track down).
Additionally introduce a BlockLiteral.SetupBlockUnsafe method, that still has
the old behavior, so that we can use it in our own (reviewed) code.
* [ObjCRuntime] Add some validation to BlockLiteral.SetupBlock.
* Use BlockLiteral.SetupBlockUnsafe instead of .SetupBlock
Use SetupBlockUnsafe in our own code, because we know our own code is using it
correctly (by passing a trampoline stored in a static field, so that the GC
doesn't free it).
* [tests] Fix xammac tests build.
- ApiTypoTest now enforces the rules defined here: https://github.com/xamarin/xamarin-macios/wiki/BINDINGS#availability-attributes-messages.
- Update all availability messages to follow new ApiTypoTest rules.
- Fix `IsObsolete` to handle ObsoletedAttribute.
- Don't apply rule 1 on Obsolete attribute.
- Allow to skip rule 4.
- Prevent use of OSX, OS X.
Rolf Bjarne Kvinge
Steve Hawley
Chris Hamons
TJ Lambert
Sebastien Pouliot
Vincent Dondain