-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Annotate System.Linq.Queryable with RDC #72899
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
Conversation
Annotates for RequiresDynamicCode, which represents AOT safety.
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-system-linq Issue DetailsAnnotates for RequiresDynamicCode, which represents AOT safety.
|
This is moving the attributes from constructors to types, which should be compatible
|
@LakshanF for review |
LakshanF
left a comment
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.
LGTM, the only FB I have is on the guidance for the user to use instead of as we generally do or perhaps in this case, warn them off this capability altogether.
src/libraries/System.Linq.Queryable/src/System.Linq.Queryable.csproj
Outdated
Show resolved
Hide resolved
| { | ||
| private static MethodInfo? s_Aggregate_TSource_2; | ||
|
|
||
| [RequiresDynamicCode("Calls System.Reflection.MethodInfo.MakeGenericMethod(params Type[])")] |
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.
This message might not be very user friendly in terms to what the problem is and how to work around it
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.
This is an internal class so I don't think we need to be more specific.
| public static class Queryable | ||
| { | ||
| internal const string InMemoryQueryableExtensionMethodsRequiresUnreferencedCode = "Enumerating in-memory collections as IQueryable can require unreferenced code because expressions referencing IQueryable extension methods can get rebound to IEnumerable extension methods. The IEnumerable extension methods could be trimmed causing the application to fail at runtime."; | ||
| internal const string InMemoryQueryableExtensionMethodsRequiresDynamicCode = "Enumerating collections as IQueryable can require creating new generic types or methods, which requires creating code at runtime. This may not work when AOT compiling."; |
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.
Any options that we can recommend to the user to use instead of?
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.
I don't know of any, and there aren't any suggestions for trimming, so I don't think we'll add any here.
Annotates for RequiresDynamicCode, which represents AOT safety.