Check async method builder generic method constraints at call-site#24322
Check async method builder generic method constraints at call-site#24322cston merged 1 commit intodotnet:dev15.6.xfrom
Conversation
There was a problem hiding this comment.
, int arity [](start = 92, length = 11)
It looks like we don't need this parameter, the arity should be available from the method #Closed
There was a problem hiding this comment.
ParseConstraints(builder, stream); [](start = 16, length = 34)
Consider keeping constraints as a separate entry in the MemberDescriptor, constraints are not really part of a signature. #Closed
There was a problem hiding this comment.
if (!MatchType(types[i], signature, ref position)) [](start = 16, length = 50)
We probably should not assume that types are always coming in the same order. #Closed
There was a problem hiding this comment.
MatchTypeParameterConstraints [](start = 21, length = 29)
If we start verifying constraints for methods, I think we should also verify them for types. #Closed
There was a problem hiding this comment.
attributes [](start = 16, length = 10)
Should we incorporate variance too? #Closed
|
I think we should consider to specify required constraints and only check that those are present rather than requiring exact match. Otherwise things might get broken for platforms that omit/add "insignificant" constraints. For example, old frameworks do not have variance constraints, but they do not matter for our scenarios. Alternatively, we should consider simply verifying constraints once we construct symbols. #Closed |
|
Done with review pass (iteration 1). #Closed |
|
@cston Since the PR is against master, I marked it as |
|
@dotnet/roslyn-compiler please review. #Resolved |
|
@dotnet-bot test windows_release_vs-integration_prtest #Resolved |
There was a problem hiding this comment.
Is it useful to also test case where Awaiter doesn't inherit INotifyCompletion?
|
@MeiChin-Tsai for approval. |
|
Approved. |
Customer scenario
The compiler currently ignores constraints on generic methods of the async method build for custom Task-like types. If those constraints are not satisfied by the awaiter or state machine for the Task-like type, the compiler will generate code does not verify and likely fails at runtime. Instead, the compiler should report compile-time errors for such cases.
Bugs this fixes
#21500
#12616
Workarounds, if any
None
Risk
Low.
Performance impact
Low.
Is this a regression from a previous update?
No.
How was the bug found?
Customer reported.