Skip to content
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

fix ignoring [MaybeNull] during type inference #66190

Closed
wants to merge 1 commit into from

Conversation

TomatorCZ
Copy link
Contributor

The fix regards the issue

Problem
The parameter field TypeWithAnnotations doesn't reflect [MaybeNull] attribute during type inference of method parameters.

Result
Different behaviour of mentioned examples in the issue. The first example behaves like this method TestA<T>(IOperation<T> operation, out T result) resulting in T = string instead of TestA<T>(IOperation<T> operation, out T? result).

@TomatorCZ TomatorCZ requested a review from a team as a code owner January 2, 2023 21:41
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jan 2, 2023
@jcouv
Copy link
Member

jcouv commented Jan 3, 2023

Nullable attributes are not meant to contribute to method type inference. That part was by-design from what I recall.
The linked issue (#50782) mentions an inconsistency between out string? discard1 and out string? _. That does seems like a bug.

@jcouv jcouv self-assigned this Jan 3, 2023
@RikkiGibson
Copy link
Contributor

I'm not familiar with the decision re: nullability attributes and type inference. It does seem to me that in the out T? param case (TestB in SharpLab), we might not be contributing nullable bounds correctly.

Just because an out string? argument contributes string for T, it should not be an exact bound, so it should still be possible to choose string? as the type argument based on new StringOperation() contributing string? (maybe an exact bound, since IOperation<T> here is invariant?)

Would be interested to discuss further offline.

@TomatorCZ
Copy link
Contributor Author

I close this PR based on offline discussion with @RikkiGibson .

MaybeNull means different thing compared with ? notation.

Part of the fix continues as a separated PR regarding different behaviour of discard expression.

@TomatorCZ TomatorCZ closed this Jan 7, 2023
@TomatorCZ TomatorCZ deleted the dev/50782_preview branch May 9, 2023 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants