[dotnet] Don't claim dynamic code is unsupported when using CoreCLR. Fixes #26430 - #26468
[dotnet] Don't claim dynamic code is unsupported when using CoreCLR. Fixes #26430#26468rolfbjarne wants to merge 3 commits into
Conversation
…out the interpreter. The $(DynamicCodeSupport) default was computed with Mono-era logic: "if the interpreter isn't enabled, everything is AOT-compiled, so there's no dynamic code". Neither $(MtouchInterpreter) nor $(UseInterpreter) has any effect on CoreCLR, so on .NET 11 - where CoreCLR is the default runtime - the condition always evaluated to true, and every iOS/tvOS/Mac Catalyst app claimed that dynamic code isn't supported. That value ends up as the 'System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported' feature switch, which ILLink then substitutes into System.Private.CoreLib as a constant 'false'. This isn't just cosmetic: * Reflection.Emit throws PlatformNotSupportedException even though CoreCLR supports it just fine. * Entity Framework Core uses the feature switch to detect NativeAOT, so any DbContext fails with "Model building is not supported when publishing with NativeAOT. Use a compiled model." in a plain Debug build. So only default $(DynamicCodeSupport) to false when actually using Mono without the interpreter, and otherwise leave the property alone and let dotnet/sdk apply its default (which is 'true'). Note that this means dynamic code paths can no longer be trimmed away, which makes apps bigger (a stock Mac Catalyst app in Release went from 21.5 MB to 39 MB in a local test). Apps that don't need dynamic code can get the previous behavior - and size - back by setting <DynamicCodeSupport>false</DynamicCodeSupport>. Also add a test for the new behavior, and move the GetRuntimeHostConfigurationOption helper to TestBaseClass so it can be shared. Fixes #26430 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: da66ea8b-c45e-4f8b-88dd-ce3d6be3123a
The $(DynamicCodeSupport) default dates back to when Mono was the only runtime for iOS/tvOS/Mac Catalyst: it was set to 'false' unless the Mono interpreter was enabled. Neither $(MtouchInterpreter) nor $(UseInterpreter) mean anything for CoreCLR, so on .NET 11 - where CoreCLR is the default runtime - we ended up claiming that dynamic code isn't supported even though it is. That makes ILLink substitute RuntimeFeature.IsDynamicCodeSupported with 'false' in the trimmed System.Private.CoreLib, which in turn breaks libraries that use this feature switch to detect NativeAOT. Entity Framework Core for instance throws "Model building is not supported when publishing with NativeAOT" in plain Debug builds. So only default to 'false' when dynamic code really isn't available: * NativeAOT. * Mono without the interpreter on iOS/tvOS/Mac Catalyst. In every other case we leave the property alone and let dotnet/sdk pick its default (which is 'true'). Fixes #26430 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: da66ea8b-c45e-4f8b-88dd-ce3d6be3123a
There was a problem hiding this comment.
Pull request overview
Updates DynamicCodeSupport defaults so CoreCLR is not treated as lacking dynamic code, while preserving false for NativeAOT and non-interpreted Mono.
Changes:
- Refines runtime-specific dynamic-code defaults.
- Adds regression tests.
- Updates size and preserved-API baselines.
Reviewed changes
Copilot reviewed 8 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Summary |
|---|---|
tests/dotnet/UnitTests/expected/TVOS-CoreCLR-R2R-size.txt |
Updates tvOS CoreCLR R2R size baseline. |
tests/dotnet/UnitTests/expected/TVOS-CoreCLR-Interpreter-size.txt |
Updates tvOS CoreCLR interpreter size baseline. |
tests/dotnet/UnitTests/expected/MacCatalyst-CoreCLR-R2R-size.txt |
Updates Mac Catalyst CoreCLR R2R size baseline. |
tests/dotnet/UnitTests/expected/MacCatalyst-CoreCLR-Interpreter-size.txt |
Updates Mac Catalyst CoreCLR interpreter size baseline. |
tests/dotnet/UnitTests/expected/iOS-CoreCLR-R2R-size.txt |
Updates iOS CoreCLR R2R size baseline. |
tests/dotnet/UnitTests/expected/iOS-CoreCLR-Interpreter-size.txt |
Updates iOS CoreCLR interpreter size baseline. |
tests/dotnet/UnitTests/expected/iOS-CoreCLR-Interpreter-preservedapis.txt |
Records newly preserved dynamic-code APIs. |
tests/dotnet/UnitTests/DynamicCodeSupportTest.cs |
Adds dynamic-code configuration regression tests. |
dotnet/targets/Xamarin.Shared.Sdk.targets |
Adjusts runtime-aware defaults; coverage is missing for non-interpreted Mono without an interpreter. |
Suppressed comments (3)
dotnet/targets/Xamarin.Shared.Sdk.targets:173
- 💡 Documentation — This makes
DynamicCodeSupporta meaningful user-facing override, with a substantial app-size tradeoff when left enabled, butdocs/building-apps/build-properties.mdhas no entry for it while documenting adjacent feature-switch properties such asDynamicRegistrationSupported,MtouchInterpreter, andUseInterpreter. Please document the platform/runtime defaults and thefalseopt-out so users can make this size-versus-dynamic-code choice intentionally.
<DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And '$(_UseNativeAot)' == 'true'">false</DynamicCodeSupport>
<DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And '$(UseMonoRuntime)' == 'true' And ( '$(MtouchInterpreter)' == '' And '$(UseInterpreter)' != 'true' ) And ('$(_PlatformName)' == 'iOS' Or '$(_PlatformName)' == 'tvOS' Or '$(_PlatformName)' == 'MacCatalyst')">false</DynamicCodeSupport>
tests/dotnet/UnitTests/DynamicCodeSupportTest.cs:74
⚠️ Test coverage — Treating a missingDynamicCodeSupportproperty as"true"means this test passes even if the SDK default becomesfalse(or the feature switch is never emitted); the helper therefore does not verify the effective value it claims to return. Validate the generated feature-switch/runtime value instead, or make the SDK-default behavior an explicit assertion.
if (!BinLog.TryFindPropertyValue (binLogPath, "DynamicCodeSupport", out var value) || string.IsNullOrEmpty (value))
return "true";
tests/dotnet/UnitTests/DynamicCodeSupportTest.cs:13
⚠️ Testing — This new matrix never setsUseMonoRuntime=true, so the changed Mono-specific default atXamarin.Shared.Sdk.targets:173is not exercised: neither Mono without an interpreter (expectedfalse) nor Mono withMtouchInterpreter/UseInterpreter(expectedtrue) is covered. Please add those cases so a regression in the legacy Mono behavior cannot pass while the CoreCLR and NativeAOT tests remain green.
public void SupportedWithCoreCLR (ApplePlatform platform, string runtimeIdentifiers)
| <!-- This should be set by dotnet/sdk instead, once https://github.com/dotnet/sdk/issues/25392 gets resolved. --> | ||
| <DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And ( '$(MtouchInterpreter)' == '' And '$(UseInterpreter)' != 'true' ) And ('$(_PlatformName)' == 'iOS' Or '$(_PlatformName)' == 'tvOS' Or '$(_PlatformName)' == 'MacCatalyst')">false</DynamicCodeSupport> | ||
| <DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And '$(_UseNativeAot)' == 'true'">false</DynamicCodeSupport> | ||
| <DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And '$(UseMonoRuntime)' == 'true' And ( '$(MtouchInterpreter)' == '' And '$(UseInterpreter)' != 'true' ) And ('$(_PlatformName)' == 'iOS' Or '$(_PlatformName)' == 'tvOS' Or '$(_PlatformName)' == 'MacCatalyst')">false</DynamicCodeSupport> |
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 |
🚀 [CI Build #282dbf5] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 260 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. [attempt 2] Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. [attempt 2] Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
The
$(DynamicCodeSupport)default dates back to when Mono was the only runtime for iOS/tvOS/Mac Catalyst: it was set tofalseunless the Mono interpreter was enabled. Neither$(MtouchInterpreter)nor$(UseInterpreter)mean anything for CoreCLR, so on .NET 11 - where CoreCLR is the default runtime - we ended up claiming that dynamic code isn't supported even though it is.That makes ILLink substitute
RuntimeFeature.IsDynamicCodeSupportedwithfalsein the trimmedSystem.Private.CoreLib, which in turn breaks libraries that use this feature switch to detect NativeAOT. Entity Framework Core for instance throws "Model building is not supported when publishing with NativeAOT. Use a compiled model." in plain Debug builds.So only default to
falsewhen dynamic code really isn't available:In every other case we leave the property alone and let dotnet/sdk pick its default (which is
true).Note that this comes at a size cost, since
System.Reflection.Emitis now kept alive: a stock Mac Catalyst app in Release goes from ~21 MB to ~39 MB. Anybody who doesn't need dynamic code can opt out with<DynamicCodeSupport>false</DynamicCodeSupport>.Fixes #26430
🤖 Pull request created by Copilot