Method Type Inference in VB #12932
Method Type Inference in VB #12932
Conversation
|
@dotnet/roslyn-compiler - please review |
|
@VSadov Is that a generic tuple? |
| If tupleLiteral.Type IsNot Nothing Then | ||
| AddTypeToGraph(argNode, isOutgoingEdge:=True) | ||
| Return | ||
| End If |
There was a problem hiding this comment.
I don't think there should be a Return here.
There was a problem hiding this comment.
It is treated the same as Exit Sub
There was a problem hiding this comment.
Right. We should not have an Exit Sub here either. That bypasses the following loop, which is required to get the correct semantics.
In reply to: 73605505 [](ancestors = 73605505)
There was a problem hiding this comment.
@gafter Thank you, that better clarifies your comment.
There was a problem hiding this comment.
Yet it passed the unit tests.
There was a problem hiding this comment.
Vlad and I met in my office to discuss the kinds of unit tests needed to demonstrate my point. For example
M((T, U) tuple) with an argument (1, 2). The expressions 1 and 2 will not propagate to give types for T and U because the following loop is skipped. I just placed a brief comment to remind him where in the code we were discussing the issue.
In reply to: 73606403 [](ancestors = 73606403)
There was a problem hiding this comment.
Right. We should always do the inference from arguments if the target is a tuple type of matching cardinality.
Only if that is not the case we should attempt to infer from the whole tuple literal.
Thanks! Good catch.!
In reply to: 73610026 [](ancestors = 73610026,73606403)
|
|
|
It is a tuple whose elements are typed as generic type parameters. In reply to: 237678618 [](ancestors = 237678618) |
|
Starting review |
| System.ValueTuple`2[System.ValueTuple`2[System.Int32,System.Object],System.ValueTuple`2[System.Int32,System.Object]] | ||
|
|
||
| ]]>) | ||
| End Sub |
There was a problem hiding this comment.
It's not directly for the PR, but I didn't find a test using tuple syntax but without referencing the ValueTuple library.
In C#, it reports ERR_PredefinedTypeMemberNotFoundInAssembly.
|
LGTM but I have to say the inference code was a bit over my head with my limited VB knowledge at this point. |
The idea is that when
(int, long)is passed to(T, U), we can infer T=int, U=long.The situation is generally more complex though since tuples have Narrowing and Widening conversions as long as underlying types allow that. For example, in the example above, if target is
(T, T)the inference will be T=longThe implementation roughly follows the same strategy as in C#. However it is plugged into a substantially different and more complex framework of type inference in VB.
Includes some fixes from the conversions PR feedback.