Implement data flow annotations on generic parameters #1245
Conversation
|
This is still missing making |
|
Looks reasonable. Would it make sense to treat |
src/linker/Linker.Steps/MarkStep.cs
Outdated
| { | ||
| while (type is TypeSpecification specification) { | ||
| if (type is GenericInstanceType git) { | ||
| MarkGenericArguments (git); | ||
| MarkGenericArguments (git, sourceLocationMember ?? (reason.Source as IMemberDefinition) ?? (reason.Source as MemberReference)?.Resolve ()); |
There was a problem hiding this comment.
I'd prefer if reason.Source was not used in this context as I have no confidence in the value and we have no tests for that
There was a problem hiding this comment.
In that case we would have to pass the new parameter on every call to MarkType and I would have to add a similar parameter to MarkMethod and also pass it everywhere.
I actually did that at first, but it turned out that in almost all cases we passed the same value to that new parameter as we do to reason.Source.
So in a way the tests for this new functionality would also validate the reason.Source.
There was a problem hiding this comment.
I propagated the source member explicitly - this spreads everywhere, so the change is large (almost all of mark step), but mostly it's just mechanical.
|
@MichalStrehovsky @marek-safar can you please take a look again - it should be complete now. |
vitek-karas
force-pushed
the
GenericParameterDataFlow
branch
from
fdd90cd
to
6f9c0a4
Compare
| methodAnnotations.TryGetAnnotation (genericParameter, out var annotation)) | ||
| return annotation.Annotation; | ||
|
|
||
| return DynamicallyAccessedMemberTypes.None; |
There was a problem hiding this comment.
nit: redundant return?
The checks are done when linker calls MarkType(TypeReference) or MarkMethod(MethodReference). The check itself is straightforward as it uses existing functionality. Added annotations in FlowAnnotations for generic parameters. Added one parameter to MarkType to better track the location where the type reference comes from. This enables much more precise reporting of warnings. On test side this makes the recognized/unrecognized attributes applicable to many more things as warnings can come from types, fields and so on.
Data flow on generic parameters makes any generic instantiation potentially problematic. So we need to start marking MakeGenericType and MakeGenericMethod as problematic as well. MakeGenericType is possible to only warn when there are generic parameters with annotations. For MakeGenericMethod we can't do anything special as we don't track MethodInfo instances.
vitek-karas
force-pushed
the
GenericParameterDataFlow
branch
from
f55f6a7
to
93bccd0
Compare
…1245) * Implement data flow annotations on generic parameters The checks are done when linker calls MarkType(TypeReference) or MarkMethod(MethodReference). The check itself is straightforward as it uses existing functionality. Added annotations in FlowAnnotations for generic parameters. Added one parameter to MarkType to better track the location where the type reference comes from. This enables much more precise reporting of warnings. On test side this makes the recognized/unrecognized attributes applicable to many more things as warnings can come from types, fields and so on. * Recognize MakeGenericType/Method and warn on them Data flow on generic parameters makes any generic instantiation potentially problematic. So we need to start marking MakeGenericType and MakeGenericMethod as problematic as well. MakeGenericType is possible to only warn when there are generic parameters with annotations. For MakeGenericMethod we can't do anything special as we don't track MethodInfo instances. Commit migrated from dotnet/linker@c75e881
The checks are done when linker calls
MarkType(TypeReference)orMarkMethod(MethodReference). The check itself is straightforward as it uses existing functionality.Added annotations in
FlowAnnotationsfor generic parameters.Added one parameter to
MarkTypeto better track the location where the type reference comes from. This enables much more precise reporting of warnings.On test side this makes the recognized/unrecognized attributes applicable to many more things as warnings can come from types, fields and so on.
Fixes #1086