-
Notifications
You must be signed in to change notification settings - Fork 554
[RGen] Generate the initWithCoder: constructor. #23803
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
[RGen] Generate the initWithCoder: constructor. #23803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements automatic generation of the initWithCoder: constructor for classes that inherit from NSCoding or implement INSCoding interfaces. The generator now detects when a class requires NSCoding support and automatically adds the required constructor, while skipping user-defined constructors with the same selector to avoid conflicts.
Key changes:
- Automatic
initWithCoder:constructor generation for NSCoding classes - Conflict resolution when user already defines the constructor
- Test coverage for both scenarios (with and without user-defined constructors)
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/rgen/Microsoft.Macios.Generator/Emitters/ClassEmitter.cs |
Core logic for detecting NSCoding classes and generating the constructor |
src/rgen/Microsoft.Macios.Generator/Emitters/Documentation.cs |
Documentation template for the generated constructor |
tests/rgen/Microsoft.Macios.Generator.Tests/Classes/ClassGenerationTests.cs |
Test data registration for NSCoding test scenarios |
| Various test data files | Input and expected output files for testing the generation |
Comments suppressed due to low confidence (1)
src/rgen/Microsoft.Macios.Generator/Emitters/ClassEmitter.cs:1
- The selector string has an extra colon. It should be
initWithCoder:notinitWithCoder::.
// Copyright (c) Microsoft Corporation.
| if (IsDirectBinding) { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); | ||
| } else { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); |
Copilot
AI
Sep 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector string has an extra colon. It should be initWithCoder: not initWithCoder::.
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); | |
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); |
| if (IsDirectBinding) { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); | ||
| } else { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); |
Copilot
AI
Sep 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector string has an extra colon. It should be initWithCoder: not initWithCoder::.
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); | |
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); |
| if (IsDirectBinding) { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); | ||
| } else { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); |
Copilot
AI
Sep 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector string has an extra colon. It should be initWithCoder: not initWithCoder::.
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); | |
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); |
| if (IsDirectBinding) { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); | ||
| } else { | ||
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); |
Copilot
AI
Sep 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector string has an extra colon. It should be initWithCoder: not initWithCoder::.
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder::"), coder.Handle), "initWithCoder:"); | |
| InitializeHandle (global::ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper_IntPtr (this.SuperHandle, Selector.GetHandle ("initWithCoder:"), coder.Handle), "initWithCoder:"); |
This comment has been minimized.
This comment has been minimized.
If a class inherits from NSCoding or implements INSCoding we generate the constructor for the 'initWithCoder:' selector. If the user defined a constructor with the selector 'initWithCoder:' selector, we will skip the user definition and will add our own. Later the analyzer will be used to let the user know that 'initWithCoder:' is automatically added.
7941f0f to
9e193b6
Compare
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.
✅ [CI Build #65f4add] Build passed (Build packages) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #65f4add] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #9e193b6] Build passed (Detect API changes) ✅Pipeline on Agent |
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.
✅ [CI Build #65f4add] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commit.NET ( No breaking changes )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #2729514] Build passed (Build macOS tests) ✅Pipeline on Agent |
✅ [CI Build #9e193b6] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #2729514] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #65f4add] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #9e193b6] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #65f4add] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #2729514] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #9e193b6] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #65f4add] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #65f4add] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #2729514] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #9e193b6] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #9e193b6] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
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.
🔥 [CI Build #65f4add] Test results 🔥Test results❌ Tests failed on VSTS: test results 3 tests crashed, 0 tests failed, 68 tests passed. Failures❌ interdependent-binding-projects tests🔥 Failed catastrophically on VSTS: test results - interdependent-binding-projects (no summary found). Html Report (VSDrops) Download ❌ linker tests🔥 Failed catastrophically on VSTS: test results - linker (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (macOS)🔥 Failed catastrophically on VSTS: test results - monotouch_macos (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
rolfbjarne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NSCoding protocol has a constructor:
Lines 3329 to 3332 in fb6a00f
| interface NSCoding { | |
| // [Abstract] | |
| [Export ("initWithCoder:")] | |
| NativeHandle Constructor (NSCoder decoder); |
So this might be better to implement by just automatically inlining constructors from protocols (which we already do for methods and properties), that way we don't have to special-case NSCoding.
| ]; | ||
|
|
||
| /// <summary> | ||
| /// Emits the NSCoding constructor (initWithCoder:) for classes that inherit from NSCoding or implement INSCoding. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inheriting from NSCoding doesn't make sense, we only care for classes that implement INSCoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's coming from the branch we depend. will update accordingly.
dalexsoto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after Rolf's 👍
|
Closing this for a more generic approach. |
If a class inherits from NSCoding or implements INSCoding we generate the constructor for the 'initWithCoder:' selector.
If the user defined a constructor with the selector 'initWithCoder:' selector, we will skip the user definition and will add our own. Later the analyzer will be used to let the user know that 'initWithCoder:' is automatically added.