[Bgen] Simplify the BindAs generated code. #22216

mandel-macaque · 2025-02-20T01:22:44Z

When a property that returns an NSNumber array is marked with the BindAs attribute, the bgen code generator uses the following pattern:

NativeHandle retvalarrtmp;
ret = ((retvalarrtmp = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle ("sourceSampleDataTrackIDs"))) == IntPtr.Zero ? null! : (NSArray.ArrayFromHandleFunc <int> (retvalarrtmp, ptr => {
  using (var num = Runtime.GetNSObject<NSNumber> (ptr)!) {
    return (int) num.Int32Value;
  }
}, false)));

This code can be simplified to use a static group method. So that it looks like:

NativeHandle retvalarrtmp;
ret = ((retvalarrtmp = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle ("sourceSampleDataTrackIDs"))) == IntPtr.Zero ? null! : (NSArray.ArrayFromHandleFunc <int> (retvalarrtmp, NSNumber.ToInt32, false)));

A seasoned C# developer would mention that in C# 10 and under using method groups (specially static ones) alocates more memory than using lambdas (lambdas were cached, static methods were not, see dotnet/roslyn#5835). Alas, this is not longer the case after dotnet/roslyn#58288 which means that the memory usage is the same allowing us to clean our generated code.

Using static group methods also opens the door to other possible improvements mentioned in the roslyn PR.

PS: This is some extra work we are doing to make the generated code simpler before we move to rgen.

When a property that returns an NSNumber array is marked with the BindAs attribute, the bgen code generator uses the following pattern: ```csharp NativeHandle retvalarrtmp; ret = ((retvalarrtmp = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle ("sourceSampleDataTrackIDs"))) == IntPtr.Zero ? null! : (NSArray.ArrayFromHandleFunc <int> (retvalarrtmp, ptr => { using (var num = Runtime.GetNSObject<NSNumber> (ptr)!) { return (int) num.Int32Value; } }, false))); ``` This code can be simplified to use a static group method. So that it looks like: ```csharp NativeHandle retvalarrtmp; ret = ((retvalarrtmp = global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle ("sourceSampleDataTrackIDs"))) == IntPtr.Zero ? null! : (NSArray.ArrayFromHandleFunc <int> (retvalarrtmp, NSNumber.ToInt32, false))); ``` A seasoned C# developer would mention that in C# 10 and under using method groups (specially static ones) alocates more memory than using lambdas (lambdas were cached, static methods were not, see dotnet/roslyn#5835). Alas, this is not longer the case after dotnet/roslyn#58288 which means that the memory usage is the same allowing us to clean our generated code. Using static group methods also opens the door to other possible improvements mentioned in the roslyn PR. PS: This is some extra work we are doing to make the generated code simpler before we move to rgen.

github-actions · 2025-02-20T01:26:49Z

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

rolfbjarne · 2025-02-20T07:48:43Z

src/Foundation/NSNumber2.cs

@@ -130,6 +130,96 @@ public static explicit operator bool (NSNumber source)
 			return source.BoolValue;
 		}

+		public static bool ToBool (NativeHandle handle)


xml docs would be nice!

src/Foundation/NSNumber2.cs

dalexsoto

+1 on xml docs

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>

1. Add missing xml comments. 2. Remove the ToEnum method which can give casting exceptions and use a lamdba in that case with a cast. For reference, the enum lambda looks like: ```csharp ptr => (global::CoreVideo.CVPixelFormatType) NSNumber.ToUInt32 (ptr) ```

vs-mobiletools-engineering-service2 · 2025-02-20T14:15:09Z

✅ [PR Build] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 4e5b7754d2ec372894cc8a4e5e8cd078e223f425 [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T14:20:50Z

✅ [PR Build] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 4e5b7754d2ec372894cc8a4e5e8cd078e223f425 [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T14:48:17Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 4e5b7754d2ec372894cc8a4e5e8cd078e223f425 [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T14:51:23Z

✅ [PR Build] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: f257db32bfb0893b2418fa1fadbb4057bac8ebfd [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T15:02:51Z

✅ API diff for current PR / commit

.NET ( No breaking changes )

iOS: vsdrops gist ( No breaking changes )
tvOS: vsdrops gist ( No breaking changes )
MacCatalyst: vsdrops gist ( No breaking changes )
macOS: vsdrops gist ( No breaking changes )

❗ API diff vs stable (Breaking changes)

.NET ( ❗ Breaking changes ❗ )

iOS: vsdrops gist ( ❗ Breaking changes ❗ )
tvOS: vsdrops gist ( ❗ Breaking changes ❗ )
MacCatalyst: vsdrops gist ( ❗ Breaking changes ❗ )
macOS: vsdrops gist ( ❗ Breaking changes ❗ )

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 4e5b7754d2ec372894cc8a4e5e8cd078e223f425 [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T15:06:18Z

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: f257db32bfb0893b2418fa1fadbb4057bac8ebfd [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T15:06:24Z

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: f257db32bfb0893b2418fa1fadbb4057bac8ebfd [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T15:07:57Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: f257db32bfb0893b2418fa1fadbb4057bac8ebfd [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T15:08:07Z

💻 [PR Build] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

✅ All tests on macOS arm64 - Mac Sequoia (15) passed.

Pipeline on Agent
Hash: f257db32bfb0893b2418fa1fadbb4057bac8ebfd [PR build]

vs-mobiletools-engineering-service2 · 2025-02-20T17:17:58Z

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: test results

0 tests crashed, 1 tests failed, 113 tests passed.

Failures

❌ dotnettests tests (MacCatalyst)

1 tests failed, 0 tests passed.

DotNet tests: TimedOut (Execution timed out after 165 minutes.)

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 10 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 8 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 4e5b7754d2ec372894cc8a4e5e8cd078e223f425 [PR build]

This changes does two things: 1. Adds a GetValue internal helper for non value backend smart enums. 2. Uses the GetValue functions as the builder for the NSArray. The new method (1) is only added to those smart enums that use a none value type backing field that looks like the following: ```csharp internal static ASAuthorizationProviderAuthorizationOperation GetValue (NativeHandle handle) { using var str = Runtime.GetNSObject<NSString> (handle); return GetValue (str); } ``` This is later used to retrieve the enum values from the handle in a BindAs decorated method/property. This is very similar to what we did in #22216 and has the same performance considerations (there is no performance change after C# 11 and later).

This changes does two things: 1. Adds a GetValue internal helper for non value backend smart enums. 2. Uses the GetValue functions as the builder for the NSArray. The new method (1) is only added to those smart enums that use a none value type backing field that looks like the following: ```csharp internal static ASAuthorizationProviderAuthorizationOperation GetValue (NativeHandle handle) { using var str = Runtime.GetNSObject<NSString> (handle); return GetValue (str); } ``` This is later used to retrieve the enum values from the handle in a BindAs decorated method/property. This is very similar to what we did in #22216 and has the same performance considerations (there is no performance change after C# 11 and later). --------- Co-authored-by: GitHub Actions Autoformatter <[email protected]>

mandel-macaque requested a review from rolfbjarne as a code owner February 20, 2025 01:22

Auto-format source code

e412906

This comment has been minimized.

Sign in to view

rolfbjarne approved these changes Feb 20, 2025

View reviewed changes

dalexsoto approved these changes Feb 20, 2025

View reviewed changes

Update src/Foundation/NSNumber2.cs

61d021b

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>

This comment has been minimized.

Sign in to view

mandel-macaque mentioned this pull request Feb 20, 2025

[Bgen] Do not return null in BindAs arrays when we have IntPtr.Zero #22217

Closed

This comment has been minimized.

Sign in to view

mandel-macaque merged commit 3076865 into main Feb 20, 2025
43 checks passed

mandel-macaque deleted the dev/mandel/nsnumber-helpers branch February 20, 2025 21:52

mandel-macaque mentioned this pull request Feb 20, 2025

[BGen] Simplify BindAs for smart enums. #22222

Merged

[Bgen] Simplify the BindAs generated code. #22216

[Bgen] Simplify the BindAs generated code. #22216

Conversation

mandel-macaque commented Feb 20, 2025

github-actions bot commented Feb 20, 2025

⚠️ Your code has been reformatted. ⚠️

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne Feb 20, 2025

Choose a reason for hiding this comment

dalexsoto left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 20, 2025

✅ [PR Build] Build passed (Build packages) ✅

vs-mobiletools-engineering-service2 commented Feb 20, 2025

✅ [PR Build] Build passed (Detect API changes) ✅

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 20, 2025

💻 [CI Build] Windows Integration Tests passed 💻

vs-mobiletools-engineering-service2 commented Feb 20, 2025

✅ [PR Build] Build passed (Build macOS tests) ✅

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 20, 2025

✅ API diff for current PR / commit

❗ API diff vs stable (Breaking changes)

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Feb 20, 2025

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

vs-mobiletools-engineering-service2 commented Feb 20, 2025

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

vs-mobiletools-engineering-service2 commented Feb 20, 2025

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

vs-mobiletools-engineering-service2 commented Feb 20, 2025

💻 [PR Build] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 20, 2025

🔥 [CI Build] Test results 🔥

Test results

Failures

❌ dotnettests tests (MacCatalyst)

Successes