Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 44 additions & 79 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@ internal partial class Binder
/// <summary>
/// For the purpose of escape verification we operate with the depth of local scopes.
/// The depth is a uint, with smaller number representing shallower/wider scopes.
/// The 0 and 1 are special scopes -
/// 0 is the "external" or "return" scope that is outside of the containing method/lambda.
/// If something can escape to scope 0, it can escape to any scope in a given method or can be returned.
/// 1 is the "parameter" or "top" scope that is just inside the containing method/lambda.
/// 0, 1 and 2 are special scopes -
/// 0 is the "calling method" scope that is outside of the containing method/lambda.
/// If something can escape to scope 0, it can escape to any scope in a given method through a ref parameter or return.
/// 1 is the "return-only" scope that is outside of the containing method/lambda.
/// If something can escape to scope 1, it can escape to any scope in a given method or can be returned, but it can't escape through a ref parameter.
/// 2 is the "current method" scope that is just inside the containing method/lambda.
/// If something can escape to scope 1, it can escape to any scope in a given method, but cannot be returned.
/// n + 1 corresponds to scopes immediately inside a scope of depth n.
/// Since sibling scopes do not intersect and a value cannot escape from one to another without
/// escaping to a wider scope, we can use simple depth numbering without ambiguity.
/// </summary>
internal const uint ExternalScope = 0;
internal const uint TopLevelScope = 1;
internal const uint ReturnOnlyScope = 1;
internal const uint TopLevelScope = 2;

// Some value kinds are semantically the same and the only distinction is how errors are reported
// for those purposes we reserve lowest 2 bits
Expand Down Expand Up @@ -719,7 +722,7 @@ private static bool CheckLocalRefEscape(SyntaxNode node, BoundLocal local, uint
return true;
}

if (escapeTo == Binder.ExternalScope)
if (escapeTo is Binder.ExternalScope or Binder.ReturnOnlyScope)
{
if (localSymbol.RefKind == RefKind.None)
{
Expand Down Expand Up @@ -794,21 +797,17 @@ private uint GetParameterValEscape(ParameterSymbol parameter)

private uint GetParameterRefEscape(ParameterSymbol parameter)
{
if (UseUpdatedEscapeRules)
{
return parameter.RefKind is RefKind.None || parameter.EffectiveScope != DeclarationScope.Unscoped ? Binder.TopLevelScope : Binder.ExternalScope;
}
else
return parameter switch
{
// byval parameters can escape to method's top level. Others can escape further.
// NOTE: "method" here means nearest containing method, lambda or local function.
return parameter.RefKind == RefKind.None ? Binder.TopLevelScope : Binder.ExternalScope;
}
{ RefKind: RefKind.None } or { EffectiveScope: not DeclarationScope.Unscoped } => Binder.TopLevelScope,
{ Type.IsRefLikeType: true } => Binder.ReturnOnlyScope,
_ => Binder.ExternalScope
};
}

private bool CheckParameterValEscape(SyntaxNode node, BoundParameter parameter, uint escapeTo, BindingDiagnosticBag diagnostics)
{
Debug.Assert(escapeTo == Binder.ExternalScope);
Debug.Assert(escapeTo is Binder.ExternalScope or Binder.ReturnOnlyScope);
if (UseUpdatedEscapeRules)
{
var parameterSymbol = parameter.ParameterSymbol;
Expand All @@ -826,22 +825,32 @@ private bool CheckParameterValEscape(SyntaxNode node, BoundParameter parameter,
}
}

private bool CheckParameterRefEscape(SyntaxNode node, BoundParameter parameter, uint escapeTo, bool checkingReceiver, BindingDiagnosticBag diagnostics)
private bool CheckParameterRefEscape(SyntaxNode node, BoundExpression parameter, ParameterSymbol parameterSymbol, uint escapeTo, bool checkingReceiver, BindingDiagnosticBag diagnostics)
{
var parameterSymbol = parameter.ParameterSymbol;

if (GetParameterRefEscape(parameterSymbol) > escapeTo)
var refSafeToEscape = GetParameterRefEscape(parameterSymbol);
if (refSafeToEscape > escapeTo)
{
var isRefScoped = parameterSymbol.EffectiveScope == DeclarationScope.RefScoped;
Debug.Assert(parameterSymbol.RefKind == RefKind.None || isRefScoped);
if (checkingReceiver)
Debug.Assert(parameterSymbol.RefKind == RefKind.None || isRefScoped || refSafeToEscape == Binder.ReturnOnlyScope);

if (parameter is BoundThisReference)
{
Error(diagnostics, isRefScoped ? ErrorCode.ERR_RefReturnScopedParameter2 : ErrorCode.ERR_RefReturnParameter2, parameter.Syntax, parameterSymbol.Name);
Error(diagnostics, ErrorCode.ERR_RefReturnStructThis, node);
return false;
}
else

#pragma warning disable format
var (errorCode, syntax) = (checkingReceiver, isRefScoped, refSafeToEscape) switch
{
Error(diagnostics, isRefScoped ? ErrorCode.ERR_RefReturnScopedParameter : ErrorCode.ERR_RefReturnParameter, node, parameterSymbol.Name);
}
(checkingReceiver: true, isRefScoped: true, _) => (ErrorCode.ERR_RefReturnScopedParameter2, parameter.Syntax),
(checkingReceiver: true, isRefScoped: false, Binder.ReturnOnlyScope) => (ErrorCode.ERR_RefReturnOnlyParameter, parameter.Syntax),
(checkingReceiver: true, isRefScoped: false, _) => (ErrorCode.ERR_RefReturnParameter2, parameter.Syntax),
(checkingReceiver: false, isRefScoped: true, _) => (ErrorCode.ERR_RefReturnScopedParameter, node),
(checkingReceiver: false, isRefScoped: false, Binder.ReturnOnlyScope) => (ErrorCode.ERR_RefReturnOnlyParameter, node),
(checkingReceiver: false, isRefScoped: false, _) => (ErrorCode.ERR_RefReturnParameter, node)
};
#pragma warning restore format
Error(diagnostics, errorCode, syntax, parameterSymbol.Name);
return false;
}

Expand Down Expand Up @@ -2362,7 +2371,7 @@ private static ErrorCode GetStandardLvalueError(BindValueKind kind)

private static ErrorCode GetStandardRValueRefEscapeError(uint escapeTo)
{
if (escapeTo == Binder.ExternalScope)
if (escapeTo is Binder.ExternalScope or Binder.ReturnOnlyScope)
{
return ErrorCode.ERR_RefReturnLvalueExpected;
}
Expand Down Expand Up @@ -2524,28 +2533,9 @@ internal uint GetRefEscape(BoundExpression expr, uint scopeOfTheContainingExpres
return ((BoundLocal)expr).LocalSymbol.RefEscapeScope;

case BoundKind.ThisReference:
Debug.Assert(this.ContainingMember() is MethodSymbol { ThisParameter: not null });

var thisref = (BoundThisReference)expr;

// "this" is an RValue, unless in a struct.
if (!thisref.Type.IsValueType)
{
break;
}

if (UseUpdatedEscapeRules)
{
if (this.ContainingMember() is MethodSymbol { ThisParameter: var thisParameter } &&
thisParameter.EffectiveScope == DeclarationScope.Unscoped)
{
return Binder.ExternalScope;
}
}

//"this" is not returnable by reference in a struct.
// can ref escape to any other level
return Binder.TopLevelScope;
var thisParam = ((MethodSymbol)this.ContainingMember()).ThisParameter;
Debug.Assert(thisParam.Type.Equals(((BoundThisReference)expr).Type, TypeCompareKind.ConsiderEverything));
return GetParameterRefEscape(thisParam);

case BoundKind.ConditionalOperator:
var conditional = (BoundConditionalOperator)expr;
Expand Down Expand Up @@ -2761,7 +2751,7 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint escapeF
case BoundKind.RefValueOperator:
// The undocumented __refvalue(tr, T) expression results in an lvalue of type T.
// for compat reasons it is not ref-returnable (since TypedReference is not val-returnable)
if (escapeTo == Binder.ExternalScope)
if (escapeTo is Binder.ExternalScope or Binder.ReturnOnlyScope)
{
break;
}
Expand All @@ -2782,41 +2772,16 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint escapeF

case BoundKind.Parameter:
var parameter = (BoundParameter)expr;
return CheckParameterRefEscape(node, parameter, escapeTo, checkingReceiver, diagnostics);
return CheckParameterRefEscape(node, parameter, parameter.ParameterSymbol, escapeTo, checkingReceiver, diagnostics);

case BoundKind.Local:
var local = (BoundLocal)expr;
return CheckLocalRefEscape(node, local, escapeTo, checkingReceiver, diagnostics);

case BoundKind.ThisReference:
Debug.Assert(this.ContainingMember() is MethodSymbol { ThisParameter: not null });

var thisref = (BoundThisReference)expr;

// "this" is an RValue, unless in a struct.
if (!thisref.Type.IsValueType)
{
break;
}

//"this" is not returnable by reference in a struct.
if (escapeTo == Binder.ExternalScope)
{
if (UseUpdatedEscapeRules)
{
if (this.ContainingMember() is MethodSymbol { ThisParameter: var thisParameter } &&
thisParameter.EffectiveScope == DeclarationScope.Unscoped)
{
// can ref escape to any other level
return true;
}
}
Error(diagnostics, ErrorCode.ERR_RefReturnStructThis, node);
return false;
}

// can ref escape to any other level
return true;
var thisParam = ((MethodSymbol)this.ContainingMember()).ThisParameter;
Debug.Assert(thisParam.Type.Equals(((BoundThisReference)expr).Type, TypeCompareKind.ConsiderEverything));
return CheckParameterRefEscape(node, expr, thisParam, escapeTo, checkingReceiver, diagnostics);

case BoundKind.ConditionalOperator:
var conditional = (BoundConditionalOperator)expr;
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private BoundStatement BindYieldReturnStatement(YieldStatementSyntax node, Bindi
BoundExpression argument = (node.Expression == null)
? BadExpression(node).MakeCompilerGenerated()
: BindValue(node.Expression, diagnostics, BindValueKind.RValue);
argument = ValidateEscape(argument, ExternalScope, isByRef: false, diagnostics: diagnostics);
argument = ValidateEscape(argument, ReturnOnlyScope, isByRef: false, diagnostics: diagnostics);

if (!argument.HasAnyErrors)
{
Expand Down Expand Up @@ -1546,7 +1546,7 @@ private BoundAssignmentOperator BindAssignment(
var rightEscape = GetRefEscape(op2, LocalScopeDepth);
if (leftEscape < rightEscape)
{
Error(diagnostics, ErrorCode.ERR_RefAssignNarrower, node, getName(op1), op2.Syntax);
Error(diagnostics, rightEscape == Binder.ReturnOnlyScope ? ErrorCode.ERR_RefAssignReturnOnly : ErrorCode.ERR_RefAssignNarrower, node, getName(op1), op2.Syntax);
op2 = ToBadExpression(op2);
}
}
Expand Down Expand Up @@ -2979,7 +2979,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, BindingDiagnosti
else
{
arg = CreateReturnConversion(syntax, diagnostics, arg, sigRefKind, retType);
arg = ValidateEscape(arg, Binder.ExternalScope, refKind != RefKind.None, diagnostics);
arg = ValidateEscape(arg, Binder.ReturnOnlyScope, refKind != RefKind.None, diagnostics);
}
}
}
Expand Down Expand Up @@ -3421,7 +3421,7 @@ static BoundBlock bindExpressionBodyAsBlockInternal(ArrowExpressionClauseSyntax
ExpressionSyntax expressionSyntax = expressionBody.Expression.CheckAndUnwrapRefExpression(diagnostics, out refKind);
BindValueKind requiredValueKind = bodyBinder.GetRequiredReturnValueKind(refKind);
BoundExpression expression = bodyBinder.BindValue(expressionSyntax, diagnostics, requiredValueKind);
expression = bodyBinder.ValidateEscape(expression, Binder.ExternalScope, refKind != RefKind.None, diagnostics);
expression = bodyBinder.ValidateEscape(expression, Binder.ReturnOnlyScope, refKind != RefKind.None, diagnostics);

return bodyBinder.CreateBlockFromExpression(expressionBody, bodyBinder.GetDeclaredLocalsForScope(expressionBody), refKind, expression, expressionSyntax, diagnostics);
}
Expand All @@ -3439,7 +3439,7 @@ public BoundBlock BindLambdaExpressionAsBlock(ExpressionSyntax body, BindingDiag
var expressionSyntax = body.CheckAndUnwrapRefExpression(diagnostics, out refKind);
BindValueKind requiredValueKind = GetRequiredReturnValueKind(refKind);
BoundExpression expression = bodyBinder.BindValue(expressionSyntax, diagnostics, requiredValueKind);
expression = ValidateEscape(expression, Binder.ExternalScope, refKind != RefKind.None, diagnostics);
expression = ValidateEscape(expression, Binder.ReturnOnlyScope, refKind != RefKind.None, diagnostics);

return bodyBinder.CreateBlockFromExpression(body, bodyBinder.GetDeclaredLocalsForScope(body), refKind, expression, expressionSyntax, diagnostics);
}
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5077,6 +5077,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_RefReturnScopedParameter2" xml:space="preserve">
<value>Cannot return by reference a member of parameter '{0}' because it is scoped to the current method</value>
</data>
<data name="ERR_RefReturnOnlyParameter" xml:space="preserve">
<value>Cannot return a parameter by reference '{0}' through a ref parameter; it can only be returned in a return statement</value>
</data>
<data name="ERR_RefReturnOnlyParameter2" xml:space="preserve">
<value>Cannot return by reference a member of parameter '{0}' through a ref parameter; it can only be returned in a return statement</value>
</data>
<data name="ERR_RefReturnLocal" xml:space="preserve">
<value>Cannot return local '{0}' by reference because it is not a ref local</value>
</data>
Expand Down Expand Up @@ -5791,6 +5797,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_RefAssignNarrower" xml:space="preserve">
<value>Cannot ref-assign '{1}' to '{0}' because '{1}' has a narrower escape scope than '{0}'.</value>
</data>
<data name="ERR_RefAssignReturnOnly" xml:space="preserve">
<value>Cannot ref-assign '{1}' to '{0}' because '{1}' can only escape the current method through a return statement.</value>
</data>
<data name="IDS_FeatureEnumGenericTypeConstraint" xml:space="preserve">
<value>enum generic type constraints</value>
</data>
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,10 @@ internal enum ErrorCode
WRN_ScopedMismatchInParameterOfTarget = 9073,
WRN_ScopedMismatchInParameterOfOverrideOrImplementation = 9074,
ERR_RefReturnScopedParameter = 9075,
ERR_RefReturnScopedParameter2 = 9076
ERR_RefReturnScopedParameter2 = 9076,
ERR_RefReturnOnlyParameter = 9077,
ERR_RefReturnOnlyParameter2 = 9078,
ERR_RefAssignReturnOnly = 9079,

#endregion

Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,9 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.WRN_ScopedMismatchInParameterOfOverrideOrImplementation:
case ErrorCode.ERR_RefReturnScopedParameter:
case ErrorCode.ERR_RefReturnScopedParameter2:
case ErrorCode.ERR_RefReturnOnlyParameter:
case ErrorCode.ERR_RefReturnOnlyParameter2:
case ErrorCode.ERR_RefAssignReturnOnly:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading