[dotnet] Don't enable InlineClassGetHandle if we're using the static registrar as a custom trimmer step.#25524
[dotnet] Don't enable InlineClassGetHandle if we're using the static registrar as a custom trimmer step.#25524rolfbjarne wants to merge 1 commit into
Conversation
…registrar as a custom trimmer step. The InlineClassGetHandle step runs before marking, and the StaticRegistrar step runs after sweeping, which causes a problem when the InlineClassGetHandle step needs the registrar to run first. This won't be a problem once we've moved out of custom trimmer steps.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the defaulting logic for InlineClassGetHandle to avoid enabling it in scenarios where the static registrar runs as a custom trimmer step (ordering mismatch with InlineClassGetHandle).
Changes:
- Tighten the condition that sets the default
InlineClassGetHandlevalue for newer target framework versions. - Explicitly exclude
Registrar=static+ custom trimmer step (PrepareAssembliesnot true) from gettingInlineClassGetHandledefaulted.
|
|
||
| <!-- Set default value for InlineClassGetHandle based on .NET version --> | ||
| <PropertyGroup Condition="'$(InlineClassGetHandle)' == '' And $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '11.0'))"> | ||
| <PropertyGroup Condition="'$(InlineClassGetHandle)' == '' And $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '11.0')) And !('$(Registrar)' == 'static' And '$(PrepareAssemblies)' != 'true')"> |
|
|
||
| <!-- Set default value for InlineClassGetHandle based on .NET version --> | ||
| <PropertyGroup Condition="'$(InlineClassGetHandle)' == '' And $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '11.0'))"> | ||
| <PropertyGroup Condition="'$(InlineClassGetHandle)' == '' And $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '11.0')) And !('$(Registrar)' == 'static' And '$(PrepareAssemblies)' != 'true')"> |
✅ [PR Build #97dee75] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #97dee75] 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 #97dee75] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #97dee75] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 36 tests failed, 157 tests passed. Failures❌ fsharp tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ interdependent-binding-projects tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests (iOS)11 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)20 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)1 tests failed, 22 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 |
The InlineClassGetHandle step runs before marking, and the StaticRegistrar
step runs after sweeping, which causes a problem when the InlineClassGetHandle
step needs the registrar to run first.
This won't be a problem once we've moved out of custom trimmer steps.