[dotnet-linker] Use [DynamicDependency] attributes instead of manual marking when preserving smart enum methods.#24934
Conversation
…marking when preserving smart enum methods. This makes it easier to move this code out of a custom linker step in the future. Contributes towards #17693.
There was a problem hiding this comment.
Pull request overview
This PR updates the dotnet-linker integration to preserve smart-enum conversion methods by injecting [DynamicDependency] attributes instead of manually marking methods in a custom mark handler, supporting the longer-term goal of removing custom linker steps (issue #17693).
Changes:
- Added a new
PreserveSmartEnumConversionsStepthat scans assemblies forBindAsAttributeusage and addsDynamicDependencyattributes to keepGetConstant/GetValue. - Refactored the existing
PreserveSmartEnumConversionsHandlerto reuse shared preservation logic. - Extended
AppBundleRewriterwith a helper to addDynamicDependencyattributes, and wired the new step intoXamarin.Shared.Sdk.targetsbehind a property toggle.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/linker/MonoTouch.Tuner/PreserveSmartEnumConversions.cs | Refactors mark handler to delegate smart-enum detection/preservation to shared helper logic. |
| tools/dotnet-linker/PreserveSmartEnumConversionsStep.cs | Introduces a new pre-mark linker step that injects DynamicDependency attributes for smart-enum conversions. |
| tools/dotnet-linker/AppBundleRewriter.cs | Adds a DynamicDependencyAttribute ctor reference and a convenience method to attach the attribute based on type/module location. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Adds a property toggle and registers the new step conditionally, keeping the old handler as fallback. |
You can also share your feedback on Copilot code review. Take the survey.
| } | ||
| } | ||
|
|
||
| protected override void ProcessAssembly(AssemblyDefinition assembly) |
| bool Mark (Tuple<MethodDefinition, MethodDefinition> pair, bool alreadyProcessed, params MethodDefinition?[] conditions) | ||
| { | ||
| if (alreadyProcessed) | ||
| return false; | ||
| Context.Annotations.Mark (pair.Item1); |
| // Copyright 2017 Xamarin Inc. | ||
|
|
||
| using System.Linq; | ||
|
|
||
| using Mono.Cecil; | ||
| using Mono.Linker; | ||
| using Mono.Linker.Steps; | ||
| using Mono.Tuner; | ||
|
|
||
| using Xamarin.Bundler; | ||
| using Xamarin.Tuner; | ||
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #2d2685d] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #2d2685d] 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.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #2d2685d] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #2d2685d] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 15 tests failed, 141 tests passed. Failures❌ monotouch tests (iOS)2 tests failed, 9 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)7 tests failed, 8 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)2 tests failed, 9 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Ventura (13) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
This makes it easier to move this code out of a custom linker step in the future.
Contributes towards #17693.