Skip to content

Commit

Permalink
Relax parameter-argument modifier matching
Browse files Browse the repository at this point in the history
  • Loading branch information
@jjonescz
jjonescz committed May 17, 2023
1 parent 030d450 commit 8c5ffab
Show file tree
Hide file tree
Showing 31 changed files with 1,285 additions and 45 deletions.
53 changes: 53 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3212,6 +3212,58 @@ private BoundExpression BindArgumentExpression(BindingDiagnosticBag diagnostics,
}

#nullable enable
private void CheckRefArguments<TMember>(
MemberResolutionResult<TMember> methodResult,
AnalyzedArguments analyzedArguments,
BindingDiagnosticBag diagnostics)
where TMember : Symbol
{
if (analyzedArguments.HasErrors)
{
return;
}

var result = methodResult.Result;
var parameters = methodResult.LeastOverriddenMember.GetParameters();

for (int arg = 0; arg < analyzedArguments.Arguments.Count; arg++)
{
if (arg >= parameters.Length)
{
// We can run out of parameters before arguments. For example: `M(__arglist(x))`.
break;
}

// Warn for `ref`/`in` or None/`ref readonly` mismatch.
var argRefKind = analyzedArguments.RefKind(arg);
if (argRefKind is RefKind.Ref or RefKind.None)
{
var warnParameterKind = argRefKind == RefKind.Ref ? RefKind.In : RefKind.RefReadOnlyParameter;
var parameter = GetCorrespondingParameter(ref result, parameters, arg);
if (parameter.RefKind == warnParameterKind)
{
if (argRefKind == RefKind.None)
{
// Argument {0} should be passed with 'ref' or 'in' keyword
diagnostics.Add(
ErrorCode.WRN_ArgExpectedRefOrIn,
analyzedArguments.Arguments[arg].Syntax,
arg + 1);
}
else
{
// Argument {0} should not be passed with the '{1}' keyword
diagnostics.Add(
ErrorCode.WRN_BadArgRef,
analyzedArguments.Arguments[arg].Syntax,
arg + 1,
argRefKind.ToArgumentDisplayString());
}
}
}
}
}

private void CoerceArguments<TMember>(
MemberResolutionResult<TMember> methodResult,
ArrayBuilder<BoundExpression> arguments,
Expand Down Expand Up @@ -6136,6 +6188,7 @@ internal bool TryPerformConstructorOverloadResolution(

if (succeededIgnoringAccessibility)
{
this.CheckRefArguments<MethodSymbol>(result.ValidResult, analyzedArguments, diagnostics);
this.CoerceArguments<MethodSymbol>(result.ValidResult, analyzedArguments.Arguments, diagnostics, receiver: null);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,7 @@ private BoundCall BindInvocationExpressionContinued(

var receiver = ReplaceTypeOrValueReceiver(methodGroup.Receiver, !method.RequiresInstanceReceiver && !invokedAsExtensionMethod, diagnostics);

this.CheckRefArguments(methodResult, analyzedArguments, diagnostics);
this.CoerceArguments(methodResult, analyzedArguments.Arguments, diagnostics, receiver);

var expanded = methodResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm;
Expand Down Expand Up @@ -2102,6 +2103,7 @@ private BoundFunctionPointerInvocation BindFunctionPointerInvocation(SyntaxNode
methodsBuilder.Free();

MemberResolutionResult<FunctionPointerMethodSymbol> methodResult = overloadResolutionResult.ValidResult;
CheckRefArguments(methodResult, analyzedArguments, diagnostics);
CoerceArguments(methodResult, analyzedArguments.Arguments, diagnostics, receiver: null);

var args = analyzedArguments.Arguments.ToImmutable();
Expand Down
62 changes: 40 additions & 22 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ private MemberAnalysisResult IsConstructorApplicableInNormalForm(

var effectiveParameters = GetEffectiveParametersInNormalForm(
constructor,
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion: false,
Expand Down Expand Up @@ -789,7 +789,7 @@ private MemberAnalysisResult IsConstructorApplicableInExpandedForm(

var effectiveParameters = GetEffectiveParametersInExpandedForm(
constructor,
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion: false,
Expand Down Expand Up @@ -3072,7 +3072,7 @@ private static bool IsUnsignedIntegralType(TypeSymbol type)

internal static void GetEffectiveParameterTypes(
MethodSymbol method,
int argumentCount,
IReadOnlyList<BoundExpression> arguments,
ImmutableArray<int> argToParamMap,
ArrayBuilder<RefKind> argumentRefKinds,
bool isMethodGroupConversion,
Expand All @@ -3084,8 +3084,8 @@ internal static void GetEffectiveParameterTypes(
{
bool hasAnyRefOmittedArgument;
EffectiveParameters effectiveParameters = expanded ?
GetEffectiveParametersInExpandedForm(method, argumentCount, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, binder, out hasAnyRefOmittedArgument) :
GetEffectiveParametersInNormalForm(method, argumentCount, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, binder, out hasAnyRefOmittedArgument);
GetEffectiveParametersInExpandedForm(method, arguments, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, binder, out hasAnyRefOmittedArgument) :
GetEffectiveParametersInNormalForm(method, arguments, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, binder, out hasAnyRefOmittedArgument);
parameterTypes = effectiveParameters.ParameterTypes;
parameterRefKinds = effectiveParameters.ParameterRefKinds;
}
Expand All @@ -3104,20 +3104,20 @@ internal EffectiveParameters(ImmutableArray<TypeWithAnnotations> types, Immutabl

private EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(
TMember member,
int argumentCount,
IReadOnlyList<BoundExpression> arguments,
ImmutableArray<int> argToParamMap,
ArrayBuilder<RefKind> argumentRefKinds,
bool isMethodGroupConversion,
bool allowRefOmittedArguments)
where TMember : Symbol
{
bool discarded;
return GetEffectiveParametersInNormalForm(member, argumentCount, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, _binder, hasAnyRefOmittedArgument: out discarded);
return GetEffectiveParametersInNormalForm(member, arguments, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, _binder, hasAnyRefOmittedArgument: out discarded);
}

private static EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(
TMember member,
int argumentCount,
IReadOnlyList<BoundExpression> arguments,
ImmutableArray<int> argToParamMap,
ArrayBuilder<RefKind> argumentRefKinds,
bool isMethodGroupConversion,
Expand All @@ -3133,7 +3133,7 @@ private static EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(
// We simulate an extra parameter for vararg methods
int parameterCount = member.GetParameterCount() + (member.GetIsVararg() ? 1 : 0);

if (argumentCount == parameterCount && argToParamMap.IsDefaultOrEmpty)
if (arguments.Count == parameterCount && argToParamMap.IsDefaultOrEmpty)
{
ImmutableArray<RefKind> parameterRefKinds = member.GetParameterRefKinds();
if (parameterRefKinds.IsDefaultOrEmpty)
Expand All @@ -3146,7 +3146,7 @@ private static EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(
ArrayBuilder<RefKind> refs = null;
bool hasAnyRefArg = argumentRefKinds.Any();

for (int arg = 0; arg < argumentCount; ++arg)
for (int arg = 0; arg < arguments.Count; ++arg)
{
int parm = argToParamMap.IsDefault ? arg : argToParamMap[arg];
// If this is the __arglist parameter, or an extra argument in error situations, just skip it.
Expand All @@ -3158,7 +3158,7 @@ private static EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(
types.Add(parameter.TypeWithAnnotations);

RefKind argRefKind = hasAnyRefArg ? argumentRefKinds[arg] : RefKind.None;
RefKind paramRefKind = GetEffectiveParameterRefKind(parameter, argRefKind, isMethodGroupConversion, allowRefOmittedArguments, binder, ref hasAnyRefOmittedArgument);
RefKind paramRefKind = GetEffectiveParameterRefKind(parameter, arguments[arg], argRefKind, isMethodGroupConversion, allowRefOmittedArguments, binder, ref hasAnyRefOmittedArgument);

if (refs == null)
{
Expand All @@ -3180,6 +3180,7 @@ private static EffectiveParameters GetEffectiveParametersInNormalForm<TMember>(

private static RefKind GetEffectiveParameterRefKind(
ParameterSymbol parameter,
BoundExpression argument,
RefKind argRefKind,
bool isMethodGroupConversion,
bool allowRefOmittedArguments,
Expand All @@ -3190,9 +3191,26 @@ private static RefKind GetEffectiveParameterRefKind(

// 'None' argument is allowed to match 'In' parameter and should behave like 'None' for the purpose of overload resolution
// unless this is a method group conversion where 'In' must match 'In'
if (!isMethodGroupConversion && argRefKind == RefKind.None && paramRefKind == RefKind.In)
// There are even more relaxations with `ref readonly` parameters feature.
if (!isMethodGroupConversion)
{
return RefKind.None;
if (paramRefKind == RefKind.In)
{
if (argRefKind == RefKind.None)
{
return RefKind.None;
}

if (argRefKind == RefKind.Ref && ((CSharpParseOptions)argument.SyntaxTree.Options).IsFeatureEnabled(MessageID.IDS_FeatureRefReadonlyParameters))
{
return RefKind.Ref;
}
}

if (paramRefKind == RefKind.RefReadOnlyParameter && argRefKind is RefKind.None or RefKind.Ref or RefKind.In)
{
return argRefKind;
}
}

// Omit ref feature for COM interop: We can pass arguments by value for ref parameters if we are calling a method/property on an instance of a COM imported type.
Expand All @@ -3209,19 +3227,19 @@ private static RefKind GetEffectiveParameterRefKind(

private EffectiveParameters GetEffectiveParametersInExpandedForm<TMember>(
TMember member,
int argumentCount,
IReadOnlyList<BoundExpression> arguments,
ImmutableArray<int> argToParamMap,
ArrayBuilder<RefKind> argumentRefKinds,
bool isMethodGroupConversion,
bool allowRefOmittedArguments) where TMember : Symbol
{
bool discarded;
return GetEffectiveParametersInExpandedForm(member, argumentCount, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, _binder, hasAnyRefOmittedArgument: out discarded);
return GetEffectiveParametersInExpandedForm(member, arguments, argToParamMap, argumentRefKinds, isMethodGroupConversion, allowRefOmittedArguments, _binder, hasAnyRefOmittedArgument: out discarded);
}

private static EffectiveParameters GetEffectiveParametersInExpandedForm<TMember>(
TMember member,
int argumentCount,
IReadOnlyList<BoundExpression> arguments,
ImmutableArray<int> argToParamMap,
ArrayBuilder<RefKind> argumentRefKinds,
bool isMethodGroupConversion,
Expand All @@ -3238,7 +3256,7 @@ private static EffectiveParameters GetEffectiveParametersInExpandedForm<TMember>
bool hasAnyRefArg = argumentRefKinds.Any();
hasAnyRefOmittedArgument = false;

for (int arg = 0; arg < argumentCount; ++arg)
for (int arg = 0; arg < arguments.Count; ++arg)
{
var parm = argToParamMap.IsDefault ? arg : argToParamMap[arg];
var parameter = parameters[parm];
Expand All @@ -3247,7 +3265,7 @@ private static EffectiveParameters GetEffectiveParametersInExpandedForm<TMember>
types.Add(parm == parameters.Length - 1 ? ((ArrayTypeSymbol)type.Type).ElementTypeWithAnnotations : type);

var argRefKind = hasAnyRefArg ? argumentRefKinds[arg] : RefKind.None;
var paramRefKind = GetEffectiveParameterRefKind(parameter, argRefKind, isMethodGroupConversion, allowRefOmittedArguments, binder, ref hasAnyRefOmittedArgument);
var paramRefKind = GetEffectiveParameterRefKind(parameter, arguments[arg], argRefKind, isMethodGroupConversion, allowRefOmittedArguments, binder, ref hasAnyRefOmittedArgument);

refs.Add(paramRefKind);
if (paramRefKind != RefKind.None)
Expand Down Expand Up @@ -3305,7 +3323,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInNormalForm<TMember>(
// To determine parameter types we use the originalMember.
EffectiveParameters originalEffectiveParameters = GetEffectiveParametersInNormalForm(
GetConstructedFrom(leastOverriddenMember),
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion,
Expand All @@ -3318,7 +3336,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInNormalForm<TMember>(
// To determine parameter types we use the originalMember.
EffectiveParameters constructedEffectiveParameters = GetEffectiveParametersInNormalForm(
leastOverriddenMember,
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion,
Expand Down Expand Up @@ -3375,7 +3393,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInExpandedForm<TMember
// To determine parameter types we use the least derived member.
EffectiveParameters originalEffectiveParameters = GetEffectiveParametersInExpandedForm(
GetConstructedFrom(leastOverriddenMember),
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion: false,
Expand All @@ -3388,7 +3406,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInExpandedForm<TMember
// To determine parameter types we use the least derived member.
EffectiveParameters constructedEffectiveParameters = GetEffectiveParametersInExpandedForm(
leastOverriddenMember,
arguments.Arguments.Count,
arguments.Arguments,
argumentAnalysis.ArgsToParamsOpt,
arguments.RefKinds,
isMethodGroupConversion: false,
Expand Down
2 changes: 1 addition & 1 deletion ...Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ private static void ReportBadArgumentError(
{
if (refParameter == RefKind.None || refParameter == RefKind.In)
{
// Argument {0} should not be passed with the {1} keyword
// Argument {0} may not be passed with the '{1}' keyword
diagnostics.Add(
ErrorCode.ERR_BadArgExtraRef,
sourceLocation,
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2881,6 +2881,18 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<data name="ERR_BadArgRef" xml:space="preserve">
<value>Argument {0} must be passed with the '{1}' keyword</value>
</data>
<data name="WRN_BadArgRef" xml:space="preserve">
<value>Argument {0} should not be passed with the '{1}' keyword</value>
</data>
<data name="WRN_BadArgRef_Title" xml:space="preserve">
<value>Argument should not be passed with the keyword</value>
</data>
<data name="WRN_ArgExpectedRefOrIn" xml:space="preserve">
<value>Argument {0} should be passed with 'ref' or 'in' keyword</value>
</data>
<data name="WRN_ArgExpectedRefOrIn_Title" xml:space="preserve">
<value>Argument should be passed with 'ref' or 'in' keyword</value>
</data>
<data name="ERR_YieldInAnonMeth" xml:space="preserve">
<value>The yield statement cannot be used inside an anonymous method or lambda expression</value>
</data>
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,8 @@ internal static RefKind GetArgumentRefKind(ImmutableArray<BoundExpression> argum
argRefKind = argRefKindsOpt[i];

Debug.Assert(argRefKind == parameters[i].RefKind ||
argRefKind == RefKindExtensions.StrictIn && parameters[i].RefKind == RefKind.In,
(argRefKind == RefKindExtensions.StrictIn && parameters[i].RefKind == RefKind.In) ||
(parameters[i].RefKind == RefKind.RefReadOnlyParameter),
"in Emit the argument RefKind must be compatible with the corresponding parameter");
}
else
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2193,6 +2193,8 @@ internal enum ErrorCode
ERR_UnsupportedPrimaryConstructorParameterCapturingRefAny = 9136,

ERR_RefReadOnlyWrongOrdering = 9501, // PROTOTYPE: Pack numbers
WRN_BadArgRef = 9502,
WRN_ArgExpectedRefOrIn = 9503,

#endregion

Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ internal static int GetWarningLevel(ErrorCode code)
switch (code)
{
case ErrorCode.WRN_AddressOfInAsync:
case ErrorCode.WRN_BadArgRef: // PROTOTYPE: Document in the markdown file.
// Warning level 8 is exclusively for warnings introduced in the compiler
// shipped with dotnet 8 (C# 12) and that can be reported for pre-existing code.
return 8;
Expand Down Expand Up @@ -530,6 +531,7 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_ParamsArrayInLambdaOnly:
case ErrorCode.WRN_CapturedPrimaryConstructorParameterPassedToBase:
case ErrorCode.WRN_UnreadPrimaryConstructorParameter:
case ErrorCode.WRN_ArgExpectedRefOrIn:
return 1;
default:
return 0;
Expand Down Expand Up @@ -2311,6 +2313,8 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_ConstantValueOfTypeExpected:
case ErrorCode.ERR_UnsupportedPrimaryConstructorParameterCapturingRefAny:
case ErrorCode.ERR_RefReadOnlyWrongOrdering:
case ErrorCode.WRN_BadArgRef:
case ErrorCode.WRN_ArgExpectedRefOrIn:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
Loading

0 comments on commit 8c5ffab

Please sign in to comment.