Skip to content

Commit

Permalink
Remove unnecessary tracking of unmatched parameters in method invocat…
Browse files Browse the repository at this point in the history 
…ion escape analysis (#62847)
  • Loading branch information
@cston
cston committed Jul 26, 2022
1 parent 32465da commit dd6f30e
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 86 deletions.
129 changes: 43 additions & 86 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1486,6 +1486,10 @@ static bool hasRefStructType(Symbol symbol)
bool isRefEscape
)
{
#if DEBUG
Debug.Assert(AllParametersConsideredInEscapeAnalysisHaveArguments(argsOpt, parameters, argsToParamsOpt));
#endif

// SPEC: (also applies to the CheckInvocationEscape counterpart)
//
// An lvalue resulting from a ref-returning method invocation e1.M(e2, ...) is ref-safe - to - escape the smallest of the following scopes:
Expand All @@ -1507,8 +1511,6 @@ bool isRefEscape
//by default it is safe to escape
uint escapeScope = Binder.ExternalScope;

ArrayBuilder<bool>? inParametersMatchedWithArgs = null;

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

RefKind effectiveRefKind = GetEffectiveRefKindAndMarkMatchedInParameter(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs, out DeclarationScope scope);
RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, out DeclarationScope scope);

// ref escape scope is the narrowest of
// - ref escape of all byref arguments
Expand All @@ -1554,25 +1556,12 @@ bool isRefEscape

if (escapeScope >= scopeOfTheContainingExpression)
{
// no longer needed
inParametersMatchedWithArgs?.Free();

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

// handle omitted optional "in" parameters if there are any
ParameterSymbol? unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);

// 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 overall result)
if (unmatchedInParameter != null && isRefEscape)
{
return scopeOfTheContainingExpression;
}

// check receiver if ref-like
if (receiver?.Type?.IsRefLikeType == true)
{
Expand Down Expand Up @@ -1605,6 +1594,10 @@ bool isRefEscape
bool isRefEscape
)
{
#if DEBUG
Debug.Assert(AllParametersConsideredInEscapeAnalysisHaveArguments(argsOpt, parameters, argsToParamsOpt));
#endif

// SPEC:
// In a method invocation, the following constraints apply:
//• If there is a ref or out argument to a ref struct type (including the receiver), with safe-to-escape E1, then
Expand All @@ -1617,8 +1610,6 @@ bool isRefEscape
receiver = null;
}

ArrayBuilder<bool>? inParametersMatchedWithArgs = null;

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

RefKind effectiveRefKind = GetEffectiveRefKindAndMarkMatchedInParameter(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs, out DeclarationScope scope);
RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, out DeclarationScope scope);

// ref escape scope is the narrowest of
// - ref escape of all byref arguments
Expand All @@ -1662,9 +1653,6 @@ bool isRefEscape

if (!valid)
{
// no longer needed
inParametersMatchedWithArgs?.Free();

ErrorCode errorCode = GetStandardCallEscapeError(checkingReceiver);

string parameterName;
Expand All @@ -1689,22 +1677,6 @@ bool isRefEscape
}
}

// handle omitted optional "in" parameters if there are any
ParameterSymbol? unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);

// 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 overall result)
if (unmatchedInParameter != null && isRefEscape)
{
var parameterName = unmatchedInParameter.Name;
if (string.IsNullOrEmpty(parameterName))
{
parameterName = unmatchedInParameter.Ordinal.ToString();
}
Error(diagnostics, GetStandardCallEscapeError(checkingReceiver), syntax, symbol, parameterName);
return false;
}

// check receiver if ref-like
if (receiver?.Type?.IsRefLikeType == true)
{
Expand Down Expand Up @@ -1828,9 +1800,6 @@ bool isRefEscape
}
}

//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
if (receiverOpt?.Type?.IsRefLikeType == true)
{
Expand Down Expand Up @@ -1881,21 +1850,49 @@ void updateEscapeTo(BoundExpression argument, RefKind refKind, uint scopeOfTheCo
};
}

#if DEBUG
private static bool AllParametersConsideredInEscapeAnalysisHaveArguments(
ImmutableArray<BoundExpression> argsOpt,
ImmutableArray<ParameterSymbol> parameters,
ImmutableArray<int> argsToParamsOpt)
{
if (parameters.IsDefaultOrEmpty) return true;

var paramsMatched = BitVector.Create(parameters.Length);
for (int argIndex = 0; argIndex < argsOpt.Length; argIndex++)
{
int paramIndex = argsToParamsOpt.IsDefault ? argIndex : argsToParamsOpt[argIndex];
paramsMatched[paramIndex] = true;
}
for (int paramIndex = 0; paramIndex < parameters.Length; paramIndex++)
{
if (!paramsMatched[paramIndex])
{
// Default arguments for params arrays are not created during
// binding (see https://github.com/dotnet/roslyn/issues/49602),
// but a params array cannot contain references or ref structs.
if (parameters[paramIndex] is not { IsParams: true, Type.TypeKind: TypeKind.Array })
{
return false;
}
}
}
return true;
}
#endif

/// <summary>
/// Gets "effective" ref kind of an argument.
/// If the ref kind is 'in', marks that that corresponding parameter was matched with a value
/// We need that to detect when there were optional 'in' parameters for which values were not supplied.
/// Gets "effective" ref kind of an argument.
///
/// NOTE: Generally we know if a formal argument is passed as ref/out/in by looking at the call site.
/// However, 'in' may also be passed as an ordinary val argument so we need to take a look at corresponding parameter, if such exists.
/// There are cases like params/vararg, when a corresponding parameter may not exist, then val cannot become 'in'.
/// </summary>
private static RefKind GetEffectiveRefKindAndMarkMatchedInParameter(
private static RefKind GetEffectiveRefKind(
int argIndex,
ImmutableArray<RefKind> argRefKindsOpt,
ImmutableArray<ParameterSymbol> parameters,
ImmutableArray<int> argsToParamsOpt,
ref ArrayBuilder<bool>? inParametersMatchedWithArgs,
out DeclarationScope scope)
{
int paramIndex = argIndex < parameters.Length ?
Expand All @@ -1909,53 +1906,13 @@ void updateEscapeTo(BoundExpression argument, RefKind refKind, uint scopeOfTheCo
if (parameters[paramIndex].RefKind == RefKind.In)
{
effectiveRefKind = RefKind.In;
inParametersMatchedWithArgs = inParametersMatchedWithArgs ?? ArrayBuilder<bool>.GetInstance(parameters.Length, fillWithValue: false);
inParametersMatchedWithArgs[paramIndex] = true;
}
}

scope = paramIndex >= 0 ? parameters[paramIndex].Scope : DeclarationScope.Unscoped;
return effectiveRefKind;
}

/// <summary>
/// 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? TryGetUnmatchedInParameterAndFreeMatchedArgs(ImmutableArray<ParameterSymbol> parameters, ref ArrayBuilder<bool>? inParametersMatchedWithArgs)
{
try
{
if (!parameters.IsDefault)
{
for (int i = 0; i < parameters.Length; i++)
{
var parameter = parameters[i];
if (parameter.IsParams)
{
break;
}

if (parameter.RefKind == RefKind.In &&
inParametersMatchedWithArgs?[i] != true &&
parameter.Type.IsRefLikeType == false)
{
return parameter;
}
}
}

return null;
}
finally
{
inParametersMatchedWithArgs?.Free();
// make sure noone uses it after.
inParametersMatchedWithArgs = null;
}
}

#nullable disable

private static ErrorCode GetStandardCallEscapeError(bool checkingReceiver)
Expand Down
43 changes: 43 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,49 @@ class Program
Diagnostic(ErrorCode.ERR_EscapeVariable, "x").WithArguments("R").WithLocation(16, 71));
}

[Fact]
public void MethodInvocation_Params()
{
var source =
@"ref struct R
{
public R(ref int i) { }
}
class Program
{
static R M1(R input, params object[] array)
{
return input;
}
static R M2()
{
int i = 42;
var r = new R(ref i);
return M1(r);
}
static R M3()
{
int i = 42;
var r = new R(ref i);
return M1(r, 0, 1, 2);
}
}";
var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics(
// (15,16): error CS8347: Cannot use a result of 'Program.M1(R, params object[])' in this context because it may expose variables referenced by parameter 'input' outside of their declaration scope
// return M1(r);
Diagnostic(ErrorCode.ERR_EscapeCall, "M1(r)").WithArguments("Program.M1(R, params object[])", "input").WithLocation(15, 16),
// (15,19): error CS8352: Cannot use variable 'r' in this context because it may expose referenced variables outside of their declaration scope
// return M1(r);
Diagnostic(ErrorCode.ERR_EscapeVariable, "r").WithArguments("r").WithLocation(15, 19),
// (21,16): error CS8347: Cannot use a result of 'Program.M1(R, params object[])' in this context because it may expose variables referenced by parameter 'input' outside of their declaration scope
// return M1(r, 0, 1, 2);
Diagnostic(ErrorCode.ERR_EscapeCall, "M1(r, 0, 1, 2)").WithArguments("Program.M1(R, params object[])", "input").WithLocation(21, 16),
// (21,19): error CS8352: Cannot use variable 'r' in this context because it may expose referenced variables outside of their declaration scope
// return M1(r, 0, 1, 2);
Diagnostic(ErrorCode.ERR_EscapeVariable, "r").WithArguments("r").WithLocation(21, 19));
}

[Fact]
public void MethodArgumentsMustMatch_01()
{
Expand Down

0 comments on commit dd6f30e

Please sign in to comment.