Skip to content

Commit

Permalink
Merge pull request #22424 from OmarTawfik/remove-ref-readonly-parameters
Browse files Browse the repository at this point in the history 
Fixes #22381 - Use in for parameters and arguments
  • Loading branch information
@VSadov
VSadov committed Sep 29, 2017
2 parents 009f92e + 48f180d commit 1fd9a04
Show file tree
Hide file tree
Showing 73 changed files with 1,341 additions and 1,243 deletions.
64 changes: 32 additions & 32 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,8 @@ private bool CheckParameterValueKind(SyntaxNode node, BoundParameter parameter,
ParameterSymbol parameterSymbol = parameter.ParameterSymbol;

// all parameters can be passed by ref/out or assigned to
// except "ref readonly" parameters, which are readonly
if (parameterSymbol.RefKind == RefKind.RefReadOnly && RequiresAssignableVariable(valueKind))
// except "in" parameters, which are readonly
if (parameterSymbol.RefKind == RefKind.In && RequiresAssignableVariable(valueKind))
{
ReportReadOnlyError(parameterSymbol, node, valueKind, checkingReceiver, diagnostics);
return false;
Expand Down Expand Up @@ -961,7 +961,7 @@ bool isRefEscape
//by default it is safe to escape
uint escapeScope = Binder.ExternalScope;

ArrayBuilder<bool> refReadOnlyParametersMatchedWithArgs = null;
ArrayBuilder<bool> inParametersMatchedWithArgs = null;

if (!argsOpt.IsDefault)
{
Expand All @@ -985,7 +985,7 @@ bool isRefEscape
goto moreArguments;
}

RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref refReadOnlyParametersMatchedWithArgs);
RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs);

// ref escape scope is the narrowest of
// - ref escape of all byref arguments
Expand All @@ -1003,20 +1003,20 @@ bool isRefEscape
if (escapeScope >= scopeOfTheContainingExpression)
{
// no longer needed
refReadOnlyParametersMatchedWithArgs?.Free();
inParametersMatchedWithArgs?.Free();

// can't get any worse
return escapeScope;
}
}
}

// handle omitted optional "ref readonly" parameters if there are any
ParameterSymbol unmatchedRefReadOnlyParameter = TryGetUnmatchedRefReadOnlyParameterAndFreeMatchedArgs(parameters, ref refReadOnlyParametersMatchedWithArgs);
// handle omitted optional "in" parameters if there are any
ParameterSymbol unmatchedInParameter = TryGetunmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);

// unmatched "ref readonly" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse)
// unmatched "in" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse)
// its val escape is ExternalScope (does not affect overal result)
if (unmatchedRefReadOnlyParameter != null && isRefEscape)
if (unmatchedInParameter != null && isRefEscape)
{
return scopeOfTheContainingExpression;
}
Expand Down Expand Up @@ -1065,7 +1065,7 @@ bool isRefEscape
receiverOpt = null;
}

ArrayBuilder<bool> refReadOnlyParametersMatchedWithArgs = null;
ArrayBuilder<bool> inParametersMatchedWithArgs = null;

if (!argsOpt.IsDefault)
{
Expand All @@ -1090,7 +1090,7 @@ bool isRefEscape
goto moreArguments;
}

RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref refReadOnlyParametersMatchedWithArgs);
RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs);

// ref escape scope is the narrowest of
// - ref escape of all byref arguments
Expand All @@ -1105,7 +1105,7 @@ bool isRefEscape
if (!valid)
{
// no longer needed
refReadOnlyParametersMatchedWithArgs?.Free();
inParametersMatchedWithArgs?.Free();

ErrorCode errorCode = GetStandardCallEscapeError(checkingReceiver);

Expand All @@ -1126,14 +1126,14 @@ bool isRefEscape
}
}

// handle omitted optional "ref readonly" parameters if there are any
ParameterSymbol unmatchedRefReadOnlyParameter = TryGetUnmatchedRefReadOnlyParameterAndFreeMatchedArgs(parameters, ref refReadOnlyParametersMatchedWithArgs);
// handle omitted optional "in" parameters if there are any
ParameterSymbol unmatchedInParameter = TryGetunmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);

// unmatched "ref readonly" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse)
// unmatched "in" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse)
// its val escape is ExternalScope (does not affect overal result)
if (unmatchedRefReadOnlyParameter != null && isRefEscape)
if (unmatchedInParameter != null && isRefEscape)
{
Error(diagnostics, GetStandardCallEscapeError(checkingReceiver), syntax, symbol, unmatchedRefReadOnlyParameter.Name);
Error(diagnostics, GetStandardCallEscapeError(checkingReceiver), syntax, symbol, unmatchedInParameter.Name);
return false;
}

Expand Down Expand Up @@ -1263,7 +1263,7 @@ bool isRefEscape
}
}

//NB: we do not care about unmatched "ref readonly" parameters here.
//NB: we do not care about unmatched "in" parameters here.
// They have "outer" val escape, so cannot be worse than escapeTo.

// check val escape of receiver if ref-like
Expand All @@ -1278,38 +1278,38 @@ bool isRefEscape
/// <summary>
/// Gets "effective" ref kind of an argument.
/// Generally we know if a formal argument is passed as ref/out by looking at the call site.
/// However, to distinguish "ref readonly" and regular "val" parameters we need to take a look at corresponding parameter, if such exists.
/// NOTE: there are cases like params/vararg, when a corresponding parameter may not exist, then it cannot be a "ref readonly".
/// However, to distinguish "in" and regular "val" parameters we need to take a look at corresponding parameter, if such exists.
/// NOTE: there are cases like params/vararg, when a corresponding parameter may not exist, then it cannot be "in".
/// </summary>
private static RefKind GetEffectiveRefKind(
int argIndex,
ImmutableArray<RefKind> argRefKindsOpt,
ImmutableArray<ParameterSymbol> parameters,
ImmutableArray<int> argsToParamsOpt,
ref ArrayBuilder<bool> refReadOnlyParametersMatchedWithArgs)
ref ArrayBuilder<bool> inParametersMatchedWithArgs)
{
var effectiveRefKind = argRefKindsOpt.IsDefault ? RefKind.None : argRefKindsOpt[argIndex];
if (effectiveRefKind == RefKind.None && argIndex < parameters.Length)
{
var paramIndex = argsToParamsOpt.IsDefault ? argIndex : argsToParamsOpt[argIndex];

if (parameters[paramIndex].RefKind == RefKind.RefReadOnly)
if (parameters[paramIndex].RefKind == RefKind.In)
{
effectiveRefKind = RefKind.RefReadOnly;
refReadOnlyParametersMatchedWithArgs = refReadOnlyParametersMatchedWithArgs ?? ArrayBuilder<bool>.GetInstance(parameters.Length, fillWithValue: false);
refReadOnlyParametersMatchedWithArgs[paramIndex] = true;
effectiveRefKind = RefKind.In;
inParametersMatchedWithArgs = inParametersMatchedWithArgs ?? ArrayBuilder<bool>.GetInstance(parameters.Length, fillWithValue: false);
inParametersMatchedWithArgs[paramIndex] = true;
}
}

return effectiveRefKind;
}

/// <summary>
/// Gets a "ref readonly" parameter for which there is no argument supplied, if such exists.
/// That indicates an optional "ref readonly" parameter. We treat it as an RValue passed by reference via a temporary.
/// Gets a "in" parameter for which there is no argument supplied, if such exists.
/// That indicates an optional "in" parameter. We treat it as an RValue passed by reference via a temporary.
/// The effective scope of such variable is the immediately containing scope.
/// </summary>
private static ParameterSymbol TryGetUnmatchedRefReadOnlyParameterAndFreeMatchedArgs(ImmutableArray<ParameterSymbol> parameters, ref ArrayBuilder<bool> refReadOnlyParametersMatchedWithArgs)
private static ParameterSymbol TryGetunmatchedInParameterAndFreeMatchedArgs(ImmutableArray<ParameterSymbol> parameters, ref ArrayBuilder<bool> inParametersMatchedWithArgs)
{
try
{
Expand All @@ -1323,8 +1323,8 @@ private static ParameterSymbol TryGetUnmatchedRefReadOnlyParameterAndFreeMatched
break;
}

if (parameter.RefKind == RefKind.RefReadOnly &&
refReadOnlyParametersMatchedWithArgs?[i] != true &&
if (parameter.RefKind == RefKind.In &&
inParametersMatchedWithArgs?[i] != true &&
parameter.Type.IsByRefLikeType == false)
{
return parameter;
Expand All @@ -1336,9 +1336,9 @@ private static ParameterSymbol TryGetUnmatchedRefReadOnlyParameterAndFreeMatched
}
finally
{
refReadOnlyParametersMatchedWithArgs?.Free();
inParametersMatchedWithArgs?.Free();
// make sure noone uses it after.
refReadOnlyParametersMatchedWithArgs = null;
inParametersMatchedWithArgs = null;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ private static void CheckRestrictedTypeReceiver(BoundExpression expression, Comp

CheckFeatureAvailability(receiverArgument.Syntax, MessageID.IDS_FeatureRefExtensionMethods, diagnostics);
}
else if (receiverParameter.RefKind == RefKind.RefReadOnly)
else if (receiverParameter.RefKind == RefKind.In)
{
CheckFeatureAvailability(receiverArgument.Syntax, MessageID.IDS_FeatureRefExtensionMethods, diagnostics);
}
Expand Down
7 changes: 2 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ internal partial class Binder
type = BindType(typeSyntax, diagnostics);
foreach (var modifier in p.Modifiers)
{
var modKind = modifier.Kind();

switch (modifier.Kind())
{
case SyntaxKind.RefKeyword:
Expand All @@ -139,9 +137,8 @@ internal partial class Binder
allValue = false;
break;

case SyntaxKind.ReadOnlyKeyword:
Debug.Assert(refKind == RefKind.Ref || syntax.HasErrors);
refKind = RefKind.RefReadOnly;
case SyntaxKind.InKeyword:
refKind = RefKind.In;
allValue = false;
break;

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ internal BoundExpression GenerateConversionForAssignment(TypeSymbol targetType,

// Parameter {0} is declared as type '{1}{2}' but should be '{3}{4}'
Error(diagnostics, ErrorCode.ERR_BadParamType, lambdaParameterLocation,
i + 1, lambdaRefKind.ToPrefix(), distinguisher.First, delegateRefKind.ToPrefix(), distinguisher.Second);
i + 1, lambdaRefKind.ToParameterPrefix(), distinguisher.First, delegateRefKind.ToParameterPrefix(), distinguisher.Second);
}
else if (lambdaRefKind != delegateRefKind)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2575,9 +2575,9 @@ private RefKind GetEffectiveParameterRefKind(ParameterSymbol parameter, RefKind
{
var paramRefKind = parameter.RefKind;

if (paramRefKind == RefKind.RefReadOnly)
if (paramRefKind == RefKind.In)
{
// "ref readonly" parameters are effectively None for the purpose of overload resolution.
// "in" parameters are effectively None for the purpose of overload resolution.
paramRefKind = RefKind.None;
}

Expand Down
14 changes: 7 additions & 7 deletions ...Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -950,17 +950,17 @@ private static bool HadLambdaConversionError(DiagnosticBag diagnostics, Analyzed
ParameterSymbol parameter = method.GetParameters()[parm];
bool isLastParameter = method.GetParameterCount() == parm + 1; // This is used to later decide if we need to try to unwrap a params array
RefKind refArg = arguments.RefKind(arg);
RefKind refParm = parameter.RefKind;
RefKind refParameter = parameter.RefKind;

if (arguments.IsExtensionMethodThisArgument(arg))
{
Debug.Assert(refArg == RefKind.None);
if (refParm == RefKind.Ref || refParm == RefKind.RefReadOnly)
if (refParameter == RefKind.Ref || refParameter == RefKind.In)
{
// For ref and ref-readonly extension methods, we omit the "ref" modifier on receiver arguments.
// Setting the correct RefKind for finding the correct diagnostics message.
// For other ref kinds, keeping it as it is to find mismatch errors.
refArg = refParm;
refArg = refParameter;
}
}

Expand All @@ -974,7 +974,7 @@ private static bool HadLambdaConversionError(DiagnosticBag diagnostics, Analyzed
{
// If the problem is that a lambda isn't convertible to the given type, also report why.
// The argument and parameter type might match, but may not have same in/out modifiers
if (argument.Kind == BoundKind.UnboundLambda && refArg == refParm)
if (argument.Kind == BoundKind.UnboundLambda && refArg == refParameter)
{
((UnboundLambda)argument).GenerateAnonymousFunctionConversionError(diagnostics, parameter.Type);
}
Expand All @@ -992,9 +992,9 @@ private static bool HadLambdaConversionError(DiagnosticBag diagnostics, Analyzed
UnwrapIfParamsArray(parameter, isLastParameter));
}
}
else if (refArg != refParm && !(refArg == RefKind.None && refParm == RefKind.RefReadOnly))
else if (refArg != refParameter && !(refArg == RefKind.None && refParameter == RefKind.In))
{
if (refParm == RefKind.None || refParm == RefKind.RefReadOnly)
if (refParameter == RefKind.None || refParameter == RefKind.In)
{
// Argument {0} should not be passed with the {1} keyword
diagnostics.Add(
Expand All @@ -1012,7 +1012,7 @@ private static bool HadLambdaConversionError(DiagnosticBag diagnostics, Analyzed
sourceLocation,
symbols,
arg + 1,
refParm.ToParameterDisplayString());
refParameter.ToParameterDisplayString());
}
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitAddress.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ private bool HasHome(BoundExpression expression, bool needWriteable)

case BoundKind.Parameter:
return !needWriteable ||
((BoundParameter)expression).ParameterSymbol.RefKind != RefKind.RefReadOnly;
((BoundParameter)expression).ParameterSymbol.RefKind != RefKind.In;

case BoundKind.Local:
// locals have home unless they are byval stack locals or ref-readonly
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ private void EmitArgument(BoundExpression argument, RefKind refKind)
EmitExpression(argument, true);
break;

case RefKind.RefReadOnly:
case RefKind.In:
var temp = EmitAddress(argument, AddressKind.ReadOnly);
AddExpressionTemp(temp);
break;
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ private BoundStatement UpdateStatement(BoundSpillSequenceBuilder builder, BoundS

case BoundKind.Call:
var call = (BoundCall)expression;
if (refKind != RefKind.None && refKind != RefKind.RefReadOnly)
if (refKind != RefKind.None && refKind != RefKind.In)
{
Debug.Assert(call.Method.RefKind != RefKind.None);
_F.Diagnostics.Add(ErrorCode.ERR_RefReturningCallAndAwait, _F.Syntax.Location, call.Method);
Expand Down Expand Up @@ -794,7 +794,7 @@ public override BoundNode VisitCall(BoundCall node)

receiver = node.ReceiverOpt;
var refKind = node.Method.ContainingType.IsReadOnly?
RefKind.RefReadOnly:
RefKind.In:
ReceiverSpillRefKind(receiver);

receiver = Spill(receiverBuilder, VisitExpression(ref receiverBuilder, receiver), refKind: refKind);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ private void CheckRefReadOnlySymbols(MethodSymbol symbol)
{
foreach (var parameter in symbol.Parameters)
{
if (parameter.RefKind == RefKind.RefReadOnly)
if (parameter.RefKind == RefKind.In)
{
foundRefReadOnly = true;
break;
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,15 @@ private static bool IsSafeForReordering(BoundExpression expression, RefKind kind
}

/// <summary>
/// patch refKinds for "readonly ref" to have effective RefReadOnly kind.
/// Patch refKinds for "in" arguments to have effective RefKind.In
/// </summary>
private static ImmutableArray<RefKind> GetEffectiveArgumentRefKinds(ImmutableArray<RefKind> argumentRefKindsOpt, ImmutableArray<ParameterSymbol> parameters)
{
ArrayBuilder<RefKind> refKindsBuilder = null;
for (int i = 0; i < parameters.Length; i++)
{
var paramRefKind = parameters[i].RefKind;
if (paramRefKind == RefKind.RefReadOnly)
if (paramRefKind == RefKind.In)
{
if (refKindsBuilder == null)
{
Expand Down Expand Up @@ -661,9 +661,9 @@ private static ImmutableArray<RefKind> GetRefKindsOrNull(ArrayBuilder<RefKind> r
BoundExpression argument = rewrittenArguments[a];
int p = (!argsToParamsOpt.IsDefault) ? argsToParamsOpt[a] : a;
RefKind refKind = argumentRefKinds.RefKinds(a);
if (refKind == RefKind.None && parameters[p].RefKind == RefKind.RefReadOnly)
if (refKind == RefKind.None && parameters[p].RefKind == RefKind.In)
{
refKind = RefKind.RefReadOnly;
refKind = RefKind.In;
}

Debug.Assert(arguments[p] == null);
Expand Down
Loading

0 comments on commit 1fd9a04

Please sign in to comment.