-
Notifications
You must be signed in to change notification settings - Fork 161
Add attributes for NativeAOT safety #1196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
But the native AOT test project produces dozens more. :(
eerhardt
added a commit
to eerhardt/aspire
that referenced
this pull request
Jun 11, 2025
Since ProxyGeneration always generates IL code using RefEmit, it will never work in native AOT'd apps. But currently it is still brought into the app because RpcMarshalableConverterFactory (used by SystemTextJsonFormatter by default) eventually calls into it. To remove references to Newtonsoft.Json, we can cut this off in AOT'd apps by checking if dynamic code isn't supported and throw an exception. This will allow ProxyGeneration to be trimmed from the final app. This isn't a perfect fix, but instead a compromise. The best fix would be to make RpcMarshalableConverterFactory work correctly in AOT'd apps.
Cut ProxyGeneration out if dynamic code isn't supported
davidfowl
pushed a commit
to dotnet/aspire
that referenced
this pull request
Jun 17, 2025
* Start the beginning of AOT'ing the Aspire.Cli. Depends on microsoft/vs-streamjsonrpc#1196 * Revert cli.csproj changes * Revert Designer.cs changes * Share state object types across CLI and Hosting. * Split the Backchannel data types across assemblies again * Revert launchSettings.json
…bility. There are a couple patterns here: - Use GetMemberWithSameMetadataDefinitionAs to reflect on generic types/methods in a statically verifiable fashion. This method only exists in .NET 6+. - Suppress warnings when using GetInterfaces when the Type is annotated as preserve 'All'. dotnet/linker#1731 - When looking for a hard-coded interface like IProgress<T>, any preserved Type that implements the interface won't have the interface trimmed. - MethodNameMap uses GetInterfaceMap, which may not work in all cases. See dotnet/runtime#89157
…te on the Type parameter.
- Use for loop to avoid Roslyn analyzer warning. - Condition the suppression on !NET10.
…bility. (#1207) * Refactor code and annotations to fix all warnings in NativeAOTCompatibility. There are a couple patterns here: - Use GetMemberWithSameMetadataDefinitionAs to reflect on generic types/methods in a statically verifiable fashion. This method only exists in .NET 6+. - Suppress warnings when using GetInterfaces when the Type is annotated as preserve 'All'. dotnet/linker#1731 - When looking for a hard-coded interface like IProgress<T>, any preserved Type that implements the interface won't have the interface trimmed. - MethodNameMap uses GetInterfaceMap, which may not work in all cases. See dotnet/runtime#89157 * Refactor TrackerHelper to not need DynamicallyAccessedMembers attribute on the Type parameter. * Respond to PR feedback - Use for loop to avoid Roslyn analyzer warning. - Condition the suppression on !NET10.
- Remove leftover comment - Fix some incorrect trimming annotations
eerhardt
approved these changes
Jun 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes LGTM.
I found a few minor pieces of feedback which I addressed in #1210.
matteo-prosperi
approved these changes
Jun 19, 2025
Minor PR feedback
eerhardt
approved these changes
Jun 19, 2025
matteo-prosperi
approved these changes
Jun 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This doesn't get StreamJsonRpc to be NativeAOT safe in any significant way, but now attributes guard the areas that are not safe, and gives us a backlog of things to improve on.