__arglist cannot have an argument passed by 'in' or 'out'#26130
__arglist cannot have an argument passed by 'in' or 'out'#26130OmarTawfik merged 3 commits intodotnet:masterfrom OmarTawfik:fix-22455-arglist-in-out
Conversation
There was a problem hiding this comment.
Does the out change need to be recorded as a breaking change? #Resolved
There was a problem hiding this comment.
Consider extracting a local for analyzedArguments.RefKind(i) for use here and in switch (refKind). #Resolved
VSadov
left a comment
There was a problem hiding this comment.
LGTM
Breaking change doc needs to be updated - for out
There was a problem hiding this comment.
throw ExceptionUtilities.UnexpectedValue(analyzedArguments.RefKind(i)); [](start = 24, length = 71)
throw ExceptionUtilities.UnexpectedValue(analyzedArguments.RefKind(i)); [](start = 24, length = 71)
I think it is better to not throw here, but instead check for kinds that we support. We support None and Ref, for everything else it is safe to report an error and I believe it is better than throwing. #Closed
There was a problem hiding this comment.
What do you suggest to put in the diagnostic message instead? the notion of "refkind" is not public or intuitive, and the current one only mentions in and out. #Resolved
There was a problem hiding this comment.
What do you suggest to put in the diagnostic message instead? the notion of "refkind" is not public or intuitive, and the current one only mentions in and out.
I suggest to go with the message you have. It covers what we have today and, once we add a new kind, we either adjust the condition of the message. #Closed
There was a problem hiding this comment.
If we fail to make an adjustment in the future, we end up with inaccurate error message, which is much better than a crash. #Closed
|
Done with review pass (iteration 2). #Closed |
|
@cston @AlekseyTs comments addressed. |
Fixes #22455
cc @VSadov @dotnet/roslyn-compiler