[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 3 commits into
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.
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.
…[Intrinsic]. Add a new custom linker step that replaces runtime `Class.GetHandle[Intrinsic]` calls with direct native symbol lookups via P/Invokes to `__Internal`. This improves startup performance and app size, and ensures the native linker can see symbol references that would otherwise only be resolved at runtime.
Three tests were failing when running with linksdk (SDK trimming enabled): - AllItemsWithNSMenuShouldAllowNull: The Menu property can be trimmed away in any link mode, not just link-all. Handle null prop with IsLinkAny instead of only IsLinkAll. - DerivedEvents_OverwriteThrows: In release+linksdk, the generated delegate-checking code can be trimmed, so the expected throw won't happen. Use IsLinkAny instead of IsLinkAll to detect this. - ExpectedLibrariesAreLoaded: With any SDK linking, frameworks that have no surviving code references won't be loaded. Skip the test when IsLinkAny, not just IsLinkAll. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rolfbjarne
force-pushed
the
dev/rolf/inline-class-gethandle
branch
from
9205925 to
2d0ba70
Compare
✅ [PR Build #2d0ba70] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #2d0ba70] 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 #2d0ba70] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
|
/review |
|
✅ .NET for Apple Platforms PR Reviewer completed successfully! |
There was a problem hiding this comment.
Review Summary
This PR adds support for inlining Class.GetHandle calls with direct native references to Objective-C classes at build time — a significant performance and size optimization for .NET apps targeting Apple platforms. The implementation follows the established pattern from InlineDlfcnMethods and is well-architected with both ILTrim and NativeAOT support.
Issues Found
❌ Error-level issues (1):
- Missing validation in
PostTrimmingProcessing.cswhenTypeMapFilePathis empty or file doesn't exist — will throwArgumentExceptionorFileNotFoundExceptionat runtime
- Missing directory creation before writing cache files in
CollectPostILTrimInformation.cs - Type map parsing could fail silently with malformed data
- Potential null reference in
InlineClassGetHandleStep.csif static registrar is not initialized
💡 Suggestions (3):
- Code organization improvements in
PathUtils.cs,FileUtils.cs, and MSBuild conditions - Minor performance optimization opportunity in
CollectPostILTrimInformation.cs
Positive Notes
✅ Well-structured documentation — Both build-properties.md and the new class-handles.md clearly explain the feature, its modes, and how it works.
✅ Good error messages — The new MX2262-MX2266 error messages are clear and actionable.
✅ Comprehensive test coverage — Added test variations for both compatibility and strict modes across ILTrim and NativeAOT paths.
✅ Code reuse — Good refactoring in AppBundleRewriter.cs to extract CreateInternalPInvoke and GetOrCreateType for use by both InlineDlfcnMethodsStep and InlineClassGetHandleStep.
✅ Proper MSBuild patterns — Correct use of Inputs/Outputs for incremental builds, per-assembly caching, and conditional target execution.
Recommendation: Request changes to address the error-level issue in PostTrimmingProcessing.cs. The warnings should also be addressed, but the suggestions are optional improvements.
Generated by .NET for Apple Platforms PR Reviewer for issue #25318 · ● 6M
github-actions
Bot
dismissed
their stale review
Superseded by updated review from same workflow.
| Condition="('$(InlineDlfcnMethods)' != '' Or '$(InlineClassGetHandle)' != '') And '$(_UseNativeAot)' != 'true'" | ||
| DependsOnTargets="_ComputeTrimmedAssemblies;_ComputePostTrimmingPaths" | ||
| Inputs="@(_TrimmedAssembly)" | ||
| Outputs="$(_ILTrimSurvivingNativeSymbolsFile)" | ||
| > |
| Condition="'$(_UseNativeAot)' == 'true'" | ||
| DependsOnTargets="_ComputePostTrimmingPaths;_CompileNativeExecutable" | ||
| Inputs="$(NativeObject)" | ||
| Outputs="$(_NativeAOTSurvivingNativeSymbolsFile)" | ||
| > |
| if (strictMode && Configuration.Application.Registrar == Bundler.RegistrarMode.Dynamic) { | ||
| Report (ErrorHelper.CreateError (Configuration.Application, 2262, null, Errors.MX2262)); | ||
| } | ||
|
|
||
| objectiveCTypeMap = DerivedLinkContext.StaticRegistrar.Types.ToDictionary (v => v.Value.ExportedName, v => v.Value); |
| var typeMap = File.ReadAllLines (TypeMapFilePath) | ||
| .Select (line => { | ||
| var parts = line.Split ('|'); | ||
| string className = ""; | ||
| string framework = ""; |
| yield return new TestData { Variation = $"Release (strict inline dlfcn, link sdk)", TestVariation = "inline-dlfcn-methods-strict|linksdk|release", Ignored = ignore }; | ||
| if (mac_supports_arm64) | ||
| yield return new TestData { Variation = $"Release (NativeAOT, .NET 11 defaults)", TestVariation = "inline-dlfcn-methods-strict|nativeaot|release", Ignored = ignore, RuntimeIdentifier = arm64_sim_runtime_identifier }; // it's necessary to specify RID, because NativeAOT defaults to building for device | ||
| yield return new TestData { Variation = $"Release (NativeAOT, .NET 11 defaults)", TestVariation = "nativeaot-net11-defaults", Ignored = ignore, RuntimeIdentifier = arm64_sim_runtime_identifier }; // it's necessary to specify RID, because NativeAOT defaults to building for device |
| DerivedLinkContext.RequiredSymbols.AddObjectiveCClass (exportedName).AddMember (type); | ||
| } | ||
|
|
||
| // Returns true if the specified type represents an Objective-C class that should referenced as a required symbol, so that the native linker doesn't link it away. |
| if (!created_types.TryGetValue (fullName, out var cachedTypeDefinition)) { | ||
| cachedTypeDefinition = module.Types.FirstOrDefault (t => t.Namespace == @namespace && t.Name == typename); | ||
| if (cachedTypeDefinition is null) { | ||
| cachedTypeDefinition = new TypeDefinition (@namespace, typename, TypeAttributes.Public | TypeAttributes.Sealed, module.TypeSystem.Object); |
🚀 [CI Build #2d0ba70] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 185 tests passed 🎉 Tests counts✅ 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 |
TODO: improve description