Avoid resolving parameter.Type in ReportParameterErrors() #63142
Conversation
| @@ -648,7 +648,7 @@ static void addERR_ParamsCantBeWithModifier(BindingDiagnosticBag diagnostics, Sy | |||
| diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, parameterSyntax.Location, parameter.Type); | |||
| } | |||
|
|
|||
| if (parameter.Scope == DeclarationScope.ValueScoped && !parameter.Type.IsErrorTypeOrRefLikeType()) | |||
| if (parameter.Scope == DeclarationScope.ValueScoped && !parameter.TypeWithAnnotations.IsRefLikeType()) | |||
There was a problem hiding this comment.
Is there a way to also check for the error type case without pulling too aggressively on lazy .Type? #Closed
There was a problem hiding this comment.
Consider adding a comment at the top of this method that pulling on parameter.Type is cycle-prone.
There was a problem hiding this comment.
Is there a way to also check for the error type case without pulling too aggressively on lazy
.Type?
We could add a similar IsErrorType() method to TypeWithAnnotations, but I'm not sure it's worth it. (Should an unresolved type always return false for IsErrorType()?)
Consider adding a comment ...
I've added a comment to the method to avoid using parameter.Type.
| @@ -648,7 +651,7 @@ static void addERR_ParamsCantBeWithModifier(BindingDiagnosticBag diagnostics, Sy | |||
| diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, parameterSyntax.Location, parameter.Type); | |||
There was a problem hiding this comment.
Is it possible for us to avoid using parameter.Type here as well?
There was a problem hiding this comment.
I think we're ok here because we've already checked that parameter.TypeWithAnnotations is a restricted type. That means the type is not a type parameter which is the only case where we'd have an unresolved type.
There was a problem hiding this comment.
Will this break when we add allow T : ref struct? No need to adjust the implementation based on something we haven't done yet, but would be good to have a sense of what our path forward would be in that case.
There was a problem hiding this comment.
Will this break when we add
allow T : ref struct?
Yes, this will break when we support ref struct types as type arguments.
Fixes #63140