[dotnet-linker] Add a trimmer step to inline calls to Class.GetHandle[Intrinsic].#25318
[dotnet-linker] Add a trimmer step to inline calls to Class.GetHandle[Intrinsic].#25318rolfbjarne wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new trimming/linking pipeline to replace some ObjCRuntime.Dlfcn and Class.GetHandle[Intrinsic] calls with direct native references, so surviving symbols/classes can be materialized as native code after trimming and preserved for the final app link.
Changes:
- Adds new linker steps and linker configuration for inlining
DlfcnandClass.GetHandlecalls, plus symbol/type collection helpers. - Adds MSBuild tasks/targets to scan trimmed outputs / NativeAOT objects, generate native source files, and compile/link those generated references.
- Adds docs and test variations/tests intended to exercise the new symbol/class inlining behavior.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/mtouch/Errors.resx | Adds new linker warning/error resource strings. |
| tools/mtouch/Errors.designer.cs | Regenerates strongly-typed resource accessors. |
| tools/linker/MonoTouch.Tuner/ProcessExportedFields.cs | Collects [Field] symbol names for compat mode. |
| tools/linker/MonoTouch.Tuner/ListExportedSymbols.cs | Refactors Objective-C type preservation and field handling. |
| tools/linker/CoreTypeMapStep.cs | Emits Objective-C class/framework/version type-map data. |
| tools/dotnet-linker/Steps/InlineDlfcnMethodsStep.cs | New step that rewrites supported Dlfcn call patterns. |
| tools/dotnet-linker/Steps/InlineClassGetHandleStep.cs | New step that rewrites Class.GetHandle[Intrinsic] calls. |
| tools/dotnet-linker/Steps/GenerateReferencesStep.cs | Minor symbol prefix usage update. |
| tools/dotnet-linker/Steps/GenerateInlinedClassGetHandleCodeStep.cs | Adds a generated-symbol-list step for post-trim processing. |
| tools/dotnet-linker/Steps/GatherFrameworksStep.cs | Broadens framework gathering to all assemblies. |
| tools/dotnet-linker/Steps/AssemblyModifierStep.cs | Makes type iteration resilient to newly added types. |
| tools/dotnet-linker/LinkerConfiguration.cs | Adds new inlining-related options and state. |
| tools/dotnet-linker/AppBundleRewriter.cs | Adds helpers for creating synthetic types/PInvokes. |
| tools/common/Symbols.cs | Exposes symbol prefix constants/shared APIs. |
| tools/common/StaticRegistrar.cs | Exposes SDK-introduced-version lookup override. |
| tools/common/PathUtils.cs | Adds directory-creation helper for file paths. |
| tools/common/MachO.cs | Adds unresolved-symbol extraction from Mach-O/static libs. |
| tools/common/Frameworks.cs | Adds framework lookup helpers and broader gathering logic. |
| tools/common/FileUtils.cs | Adds a write-if-different helper for generated files. |
| tools/common/CompilerFlags.cs | Updates symbol prefix usage. |
| tests/xharness/Jenkins/TestVariationsFactory.cs | Adds new CI test variations for the inlining modes. |
| tests/monotouch-test/ObjCRuntime/DlfcnTest.cs | Adds regression coverage for inlined Dlfcn scenarios. |
| tests/monotouch-test/Foundation/AttributedStringTest.cs | Removes obsolete missing-symbol assertions. |
| tests/monotouch-test/dotnet/shared.csproj | Adds ignored/reference native symbols for tests. |
| tests/monotouch-test/CoreGraphics/GeometryTest.cs | Refactors CGRect symbol lookup test helper. |
| tests/monotouch-test/CoreFoundation/ProxyTest.cs | Removes obsolete missing-symbol assertions. |
| tests/common/test-variations.csproj | Declares new build/test variation names and properties. |
| src/ObjCRuntime/Registrar.cs | Exposes exported Objective-C type-name lookup helpers. |
| msbuild/Xamarin.MacDev.Tasks/Xamarin.MacDev.Tasks.csproj | Includes shared symbol code and new task files. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/PostTrimmingProcessing.cs | Generates post-trim native source files. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/ComputeNativeAOTSurvivingNativeSymbols.cs | Filters NativeAOT unresolved symbols into surviving lists. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/CompileNativeCode.cs | Ensures output directories exist before compilation. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/CollectUnresolvedNativeSymbols.cs | Collects unresolved symbols from NativeAOT outputs. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/CollectPostILTrimInformation.cs | Scans trimmed assemblies for surviving generated P/Invokes. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Wires new linker parameters, steps, and post-trim targets. |
| dotnet/targets/Xamarin.Shared.Sdk.props | Adds default values for the new inlining properties. |
| docs/code/native-symbols.md | Documents the Dlfcn native-symbol pipeline. |
| docs/code/class-handles.md | Documents the Class.GetHandle pipeline. |
| docs/building-apps/build-properties.md | Documents InlineDlfcnMethods. |
Files not reviewed (1)
- tools/mtouch/Errors.designer.cs: Language not supported
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.
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
force-pushed
the
dev/rolf/inline-class-gethandle
branch
from
83c1324 to
556a21c
Compare
|
/review |
|
✅ .NET for Apple Platforms PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds a new trimmer step to inline calls to Class.GetHandle[Intrinsic], converting them to direct P/Invoke calls that reference Objective-C classes natively. This optimization improves runtime performance and enables better native linking.
Critical Issues Found
🔴 Blocking Issues
-
Typo in MSBuild property name (dotnet/targets/Xamarin.Shared.Sdk.props:111):
TargetFrameworkVersionis split as "TargetFr ameworkVersion" with a space, breaking the condition that sets default values forInlineClassGetHandle. This will cause the feature to not activate correctly. -
Placeholder error codes (tools/dotnet-linker/Steps/InlineClassGetHandleStep.cs:41, 114, 119, 127, 132): Multiple error messages use placeholder code
9999with "Copilot:" comments indicating they need proper error codes and resource strings. These must be replaced with actual error codes before merging.
⚠️ Non-Blocking Issues
-
Typo in parameter name (PostTrimmingProcessing.cs:112):
filterObjetiveCClassesshould befilterObjectiveCClasses(missing 'c'). While this doesn't affect functionality, it should be corrected for code quality. -
Missing optimization (CollectPostILTrimInformation.cs:124): Comment indicates an unimplemented early-exit optimization for assemblies without
__Internalmodule references. -
Missing validation in typeMap parsing (PostTrimmingProcessing.cs:156): The typeMap parsing doesn't validate that required fields exist, potentially creating entries with empty keys.
-
Potential KeyNotFoundException (PostTrimmingProcessing.cs:182):
ToDictionarycall may throw if the typeMap file contains duplicate class names. Consider using a more defensive approach.
Architecture & Design
The overall design is sound:
- Clear separation of concerns between trimming steps, MSBuild tasks, and code generation
- Good use of caching for incremental builds
- Appropriate handling of both ILTrim and NativeAOT paths
- Documentation file (class-handles.md) provides good context
Recommendation
Request Changes - The typo in TargetFrameworkVersion and placeholder error codes must be fixed before this can be merged, as they will cause the feature to malfunction.
Generated by .NET for Apple Platforms PR Reviewer for issue #25318 · ● 1.6M
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
force-pushed
the
dev/rolf/inline-class-gethandle
branch
2 times, most recently
from
645a011 to
7003552
Compare
Add two attributes to specify whether an API is available in the simulator:
* [UnsupportedSimulator ("<platform>")]: API is not available.
* [SupportedSimulator ("<platform>[osversion]")]: API is available, optionally
only in the specified OS version or not.
If no attribute is found, then the API is available in the simulator.
Unlike the normal availability attributes, specifying simulator availability for
one platform does not mean anything for any other platforms.
rolfbjarne
force-pushed
the
dev/rolf/inline-class-gethandle
branch
from
7003552 to
34c7a93
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.
✅ [PR Build #a51653c] 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.
✅ [PR Build #a51653c] Build passed (Build packages) ✅Pipeline on Agent |
✅ 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 |
✅ [PR Build #a51653c] 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.
🔥 [CI Build #a51653c] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 44 tests failed, 132 tests passed. Failures❌ linker tests22 tests failed, 22 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)18 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)2 tests failed, 16 tests passed.Failed tests
Html Report (VSDrops) Download ❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download ❌ xcframework tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
No description provided.