Update linker and analyzer to handle DAM on indexable property parameters#3081
Update linker and analyzer to handle DAM on indexable property parameters#3081jtschuster merged 8 commits intodotnet:mainfrom
Conversation
Use last index of set parameters - don't assume one param
…RC2 SDK. Took the versions from dotnet/runtime.
…ePropsAnalyzer
| paramAnnotations[paramAnnotations.Length - 1] = annotation; | ||
| annotatedMethods.Add (new MethodAnnotations (setMethod, paramAnnotations, DynamicallyAccessedMemberTypes.None, null)); | ||
| } | ||
| if (setterAnnotation is not null) |
There was a problem hiding this comment.
Can this logic be replaced with the following?
setterAnnotation?.ParameterAnnotations[^1] = annotation;There was a problem hiding this comment.
I think we could, but that makes me wonder if we should actually make MethodAnnotations.ParameterAnnotations an ImmutableArray<DynamicallyAccessedMembers> instead of just a mutable array. The fields are already readonly, so I think the intent is that Annotations should be immutable data structures.
There was a problem hiding this comment.
In this case I think it is still useful to be able to modify them in-place as we are building the annotations. The usual pattern is to have some kind of mutable builder, which produces an immutable output at the end. I think since MethodAnnotations is private to FlowAnnotations, this kind of does that - after building annotations, they should only be accessed (and never mutated) through FlowAnnotations.
But I agree that since the fields are readonly there's an inconsistency. I am fine with it either way - just a suggestion since I thought the mutating version was more readable here.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
…ters (dotnet/linker#3081) Don't assume there is only one parameter for set and none for get. Allow DAM on the index parameters of properties. Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Co-authored-by: Sven Boemer <sbomer@gmail.com> Commit migrated from dotnet/linker@4db6ac9
…ters (dotnet/linker#3081) Don't assume there is only one parameter for set and none for get. Allow DAM on the index parameters of properties. Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Co-authored-by: Sven Boemer <sbomer@gmail.com> Commit migrated from dotnet/linker@4db6ac9
In the analyzer, IPropertyReference's may be indexable, which have additional parameters that must be passed to HandleCallAction.