Fix generic parameter validation in MakeGenericMethod and similar by vitek-karas · Pull Request #1983 · dotnet/linker

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas · 2021-04-21T12:12:26Z

The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

This should fix the problem in dotnet/runtime#51558 (comment).

…hod. The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator).

vitek-karas · 2021-04-21T12:12:58Z

@jkoritzinsky

marek-safar · 2021-04-21T12:43:45Z

+ bool hasRequirements = false;
+ foreach (var genericParameter in genericParameters) {
+ if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None)
+ hasRequirements = true;


missing break ?

…tnet/linker#1983) The validator must ignore methods which don't have any requirements. So even if the passed in array doesn't match the method, it needs to not warn in such cases. This is because we can't guarantee we always find the exact method info to work on, instead we compensate by considering potentially multiple candidates. Some of which may not even be generic, or may not have any requirements and so on. In such cases the validator should only warn if there's potentially a real problem - not if the reflection API will fail (that's a non-failure for the validator). Commit migrated from dotnet/linker@190c08c

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

vitek-karas added 2 commits April 21, 2021 04:59

Fix a test
a08d2bd

vitek-karas added the area-Linker: DataFlow label Apr 21, 2021

vitek-karas added this to the .NET 6.0 milestone Apr 21, 2021

vitek-karas requested a review from MichalStrehovsky April 21, 2021 12:12

vitek-karas requested a review from marek-safar as a code owner April 21, 2021 12:12

vitek-karas self-assigned this Apr 21, 2021

vitek-karas mentioned this pull request Apr 21, 2021
[main] Update dependencies from mono/linker dotnet/runtime#51558
Merged

marek-safar reviewed Apr 21, 2021
View reviewed changes

marek-safar approved these changes Apr 21, 2021
View reviewed changes

PR feedback
81fd08c

vitek-karas merged commit 190c08c into dotnet:mainApr 21, 2021

vitek-karas deleted the FixGenericParameterValidation branch April 21, 2021 14:33

Uh oh!

Conversation

vitek-karas commented Apr 21, 2021

Uh oh!

vitek-karas commented Apr 21, 2021

Uh oh!

marek-safarApr 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vitek-karas commented Apr 21, 2021

Uh oh!

vitek-karas commented Apr 21, 2021

Uh oh!

marek-safarApr 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vitek-karas commented Apr 21, 2021

Uh oh!

vitek-karas commented Apr 21, 2021

Uh oh!

marek-safarApr 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vitek-karas commented Apr 21, 2021

Uh oh!

vitek-karas commented Apr 21, 2021

Uh oh!

marek-safarApr 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vitek-karas commented Apr 21, 2021

Uh oh!

vitek-karas commented Apr 21, 2021

Uh oh!

marek-safarApr 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels