[ILLink] stub -> throw PNSE #123326
[ILLink] stub -> throw PNSE #123326
Conversation
pavelsavara
added
the
area-Tools-ILLink
dotnet-policy-service
bot
added
the
linkable-framework
pavelsavara
added
the
size-reduction
|
Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar |
There was a problem hiding this comment.
Pull request overview
This PR adds support for automatically converting methods to throw PlatformNotSupportedException (PNSE) to enable better code trimming for browser targets. The feature activates when methods are explicitly marked with body="pnse" in XML substitution files, or when using the --target-os browser command-line argument with methods annotated with [UnsupportedOSPlatform("browser")].
Changes:
- Added new
MethodAction.ConvertToPNSEenum value and corresponding body rewriting logic - Implemented
--target-oscommand-line argument to enable OS-specific substitutions - Integrated automatic PNSE substitution for browser builds in MSBuild targets
- Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| MethodAction.cs | Added new ConvertToPNSE enum value for PNSE body substitution |
| LinkContext.cs | Added TargetOS property to track target platform |
| KnownMembers.cs | Added PlatformNotSupportedExceptionCtor tracking and validation |
| Driver.cs | Implemented --target-os command-line argument parsing and help text |
| MarkStep.cs | Added logic to detect UnsupportedOSPlatform attributes and mark methods/properties for PNSE conversion |
| CodeRewriterStep.cs | Implemented PNSE body rewriting logic to replace method bodies |
| CheckSuppressionsStep.cs | Updated to skip suppression gathering for ConvertToPNSE methods |
| BodySubstitutionParser.cs | Added parsing support for body="pnse" in XML substitutions |
| WellKnownType.cs | Added System.PlatformNotSupportedException to well-known types (with bug) |
| CompatibilitySuppressions.xml | Added compatibility suppressions for new API surface |
| Microsoft.NET.ILLink.targets | Enabled --target-os browser for browser targets |
| sfx-finish.proj | Enabled --target-os browser for shared framework trimming |
| System.Threading.csproj | Enabled --target-os browser for System.Threading library |
| data-formats.md | Added documentation for the new pnse body substitution feature |
| UnsupportedOSPlatformSubstitution.cs | Added comprehensive test case for the new feature |
| SubstitutionsTests.g.cs | Auto-generated test harness for new test case |
| ### Throw PlatformNotSupportedException | ||
|
|
||
|
|
||
| Entire method body is replaces with `throw new PlatformNotSupportedException()` instruction when method is referenced. |
There was a problem hiding this comment.
Spelling error: "replaces" should be "replaced" to match the grammatical structure and be consistent with similar documentation in the file (see line 244 for the same error pattern).
| bool TryMarkMethodForUnsupportedPlatform(MethodDefinition method) | ||
| { | ||
| if (Context.TargetOS is null) | ||
| return false; | ||
|
|
||
| // Check method-level attributes | ||
| if (HasMatchingUnsupportedOSPlatformAttribute(method)) | ||
| { | ||
| Annotations.SetAction(method, MethodAction.ConvertToPNSE); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks if the property has [UnsupportedOSPlatform] attribute matching the target OS | ||
| /// and marks its getter/setter for PNSE conversion if so. | ||
| /// </summary> | ||
| void TryMarkPropertyForUnsupportedPlatform(PropertyDefinition property) | ||
| { | ||
| if (Context.TargetOS is null) | ||
| return; | ||
|
|
||
| if (!HasMatchingUnsupportedOSPlatformAttribute(property)) | ||
| return; | ||
|
|
||
| if (property.GetMethod is not null) | ||
| Annotations.SetAction(property.GetMethod, MethodAction.ConvertToPNSE); | ||
|
|
||
| if (property.SetMethod is not null) | ||
| Annotations.SetAction(property.SetMethod, MethodAction.ConvertToPNSE); | ||
| } |
There was a problem hiding this comment.
The TryMarkMethodForUnsupportedPlatform and TryMarkPropertyForUnsupportedPlatform methods unconditionally set the method action to ConvertToPNSE without checking if an action has already been set (e.g., from XML substitutions). This could overwrite explicit XML substitution directives. Consider checking the current action and only setting ConvertToPNSE if the action is MethodAction.Nothing or MethodAction.Parse to respect explicit substitution configuration.
…pe.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
Goal is to make assemblies for browser smaller by automatically trimming code that was kept alive by the unsupported methods.
body="pnse"inILLink.Substitutions.xml--target-os browserand the method is marked with[UnsupportedOSPlatform ("browser")]attribute