Skip to content

Commit

Permalink
Adjust calculation of invocation escape when return type is ref to re…
Browse files Browse the repository at this point in the history 
…f struct (#65857)
  • Loading branch information
@RikkiGibson
RikkiGibson authored Dec 21, 2022
1 parent e17d6ad commit 76cd2f1
Show file tree
Hide file tree
Showing 4 changed files with 484 additions and 48 deletions.
79 changes: 55 additions & 24 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1729,25 +1729,47 @@ private uint GetInvocationEscapeWithUpdatedRules(
isRefEscape,
ignoreArglistRefKinds: true, // https://github.com/dotnet/roslyn/issues/63325: for compatibility with C#10 implementation.
argsAndParamsAll);
foreach (var argAndParam in argsAndParamsAll)
{
var argument = argAndParam.Argument;
uint argEscape = argAndParam.IsRefEscape ?
GetRefEscape(argument, scopeOfTheContainingExpression) :
GetValEscape(argument, scopeOfTheContainingExpression);

escapeScope = Math.Max(escapeScope, argEscape);
if (escapeScope >= scopeOfTheContainingExpression)
var returnsRefToRefStruct = ReturnsRefToRefStruct(symbol);
foreach (var (param, argument, _, isArgumentRefEscape) in argsAndParamsAll)
{
// SPEC:
// If `M()` does return ref-to-ref-struct, the *safe-to-escape* is the same as the *safe-to-escape* of all arguments which are ref-to-ref-struct. It is an error if there are multiple arguments with different *safe-to-escape* because of *method arguments must match*.
// If `M()` does return ref-to-ref-struct, the *ref-safe-to-escape* is the narrowest *ref-safe-to-escape* contributed by all arguments which are ref-to-ref-struct.
//
if (!returnsRefToRefStruct
|| (param is null or { RefKind: not RefKind.None, Type.IsRefLikeType: true } && isArgumentRefEscape == isRefEscape))
{
// can't get any worse
break;
uint argEscape = isArgumentRefEscape ?
GetRefEscape(argument, scopeOfTheContainingExpression) :
GetValEscape(argument, scopeOfTheContainingExpression);

escapeScope = Math.Max(escapeScope, argEscape);
if (escapeScope >= scopeOfTheContainingExpression)
{
// can't get any worse
break;
}
}
}
argsAndParamsAll.Free();

return escapeScope;
}

private static bool ReturnsRefToRefStruct(Symbol symbol)
{
var method = symbol switch
{
MethodSymbol m => m,
// We are only getting the method in order to handle a special condition where the method returns by-ref.
// It is an error for a property to have a setter and return by-ref, so we only bother looking for a getter here.
PropertySymbol p => p.GetMethod,
_ => null
};
return method is { RefKind: not RefKind.None, ReturnType.IsRefLikeType: true };
}

/// <summary>
/// Validates whether given invocation can allow its results to escape from <paramref name="escapeFrom"/> level to <paramref name="escapeTo"/> level.
/// The result indicates whether the escape is possible.
Expand Down Expand Up @@ -1869,24 +1891,33 @@ private bool CheckInvocationEscapeWithUpdatedRules(
isRefEscape,
ignoreArglistRefKinds: true, // https://github.com/dotnet/roslyn/issues/63325: for compatibility with C#10 implementation.
argsAndParamsAll);
foreach (var argAndParam in argsAndParamsAll)
{
var argument = argAndParam.Argument;
bool valid = argAndParam.IsRefEscape ?
CheckRefEscape(argument.Syntax, argument, escapeFrom, escapeTo, false, diagnostics) :
CheckValEscape(argument.Syntax, argument, escapeFrom, escapeTo, false, diagnostics);

if (!valid)
var returnsRefToRefStruct = ReturnsRefToRefStruct(symbol);
foreach (var (param, argument, _, isArgumentRefEscape) in argsAndParamsAll)
{
// SPEC:
// If `M()` does return ref-to-ref-struct, the *safe-to-escape* is the same as the *safe-to-escape* of all arguments which are ref-to-ref-struct. It is an error if there are multiple arguments with different *safe-to-escape* because of *method arguments must match*.
// If `M()` does return ref-to-ref-struct, the *ref-safe-to-escape* is the narrowest *ref-safe-to-escape* contributed by all arguments which are ref-to-ref-struct.
//
if (!returnsRefToRefStruct
|| (param is null or { RefKind: not RefKind.None, Type.IsRefLikeType: true } && isArgumentRefEscape == isRefEscape))
{
// For consistency with C#10 implementation, we don't report an additional error
// for the receiver. (In both implementations, the call to Check*Escape() above
// will have reported a specific escape error for the receiver though.)
if ((object)((argument as BoundCapturedReceiverPlaceholder)?.Receiver ?? argument) != receiver)
bool valid = isArgumentRefEscape ?
CheckRefEscape(argument.Syntax, argument, escapeFrom, escapeTo, false, diagnostics) :
CheckValEscape(argument.Syntax, argument, escapeFrom, escapeTo, false, diagnostics);

if (!valid)
{
ReportInvocationEscapeError(syntax, symbol, argAndParam.Parameter, checkingReceiver, diagnostics);
// For consistency with C#10 implementation, we don't report an additional error
// for the receiver. (In both implementations, the call to Check*Escape() above
// will have reported a specific escape error for the receiver though.)
if ((object)((argument as BoundCapturedReceiverPlaceholder)?.Receiver ?? argument) != receiver)
{
ReportInvocationEscapeError(syntax, symbol, param, checkingReceiver, diagnostics);
}
result = false;
break;
}
result = false;
break;
}
}
argsAndParamsAll.Free();
Expand Down
4 changes: 0 additions & 4 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10944,12 +10944,8 @@ static ref Span<int> ReturnPtrByRef(delegate*<ref Span<int>, ref Span<int>> ptr)
Diagnostic(ErrorCode.WRN_RefReturnLocal, "span").WithArguments("span").WithLocation(10, 28)
);

// delegate*<,> parameter is implicitly scoped ref in C#11.
comp = CreateCompilationWithSpan(source, options: TestOptions.UnsafeReleaseExe);
comp.VerifyDiagnostics(
// (10,28): warning CS9080: Use of variable 'span' in this context may expose referenced variables outside of their declaration scope
// return ref ptr(ref span);
Diagnostic(ErrorCode.WRN_EscapeVariable, "span").WithArguments("span").WithLocation(10, 28),
// (10,28): warning CS9091: This returns local 'span' by reference but it is not a ref local
// return ref ptr(ref span);
Diagnostic(ErrorCode.WRN_RefReturnLocal, "span").WithArguments("span").WithLocation(10, 28)
Expand Down
21 changes: 11 additions & 10 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1381,17 +1381,18 @@ class Program
static Span<int> Test1()
{
var s1 = ReturnsSpan({{outVarDeclaration}});
return s1; // 1
return s1;
}
static Span<int> Test2()
{
ref var s2 = ref ReturnsSpan({{outVarDeclaration}});
s2 = stackalloc int[1];
return s2; // 2
s2 = stackalloc int[1]; // 1
return s2;
}
static void Test3()
{
ReturnsSpan({{outVarDeclaration}}) = stackalloc int[1];
ReturnsSpan({{outVarDeclaration}}) =
stackalloc int[1]; // 2
}
static ref Span<int> ReturnsSpan([UnscopedRef] out Span<int> x)
{
Expand All @@ -1402,12 +1403,12 @@ static ref Span<int> ReturnsSpan([UnscopedRef] out Span<int> x)
""";
var comp = CreateCompilationWithMscorlibAndSpan(new[] { source, UnscopedRefAttributeDefinition });
comp.VerifyDiagnostics(
// (8,16): error CS8352: Cannot use variable 's1' in this context because it may expose referenced variables outside of their declaration scope
// return s1; // 1
Diagnostic(ErrorCode.ERR_EscapeVariable, "s1").WithArguments("s1").WithLocation(8, 16),
// (14,16): error CS8352: Cannot use variable 's2' in this context because it may expose referenced variables outside of their declaration scope
// return s2; // 2
Diagnostic(ErrorCode.ERR_EscapeVariable, "s2").WithArguments("s2").WithLocation(14, 16));
// (13,14): error CS8353: A result of a stackalloc expression of type 'Span<int>' cannot be used in this context because it may be exposed outside of the containing method
// s2 = stackalloc int[1]; // 1
Diagnostic(ErrorCode.ERR_EscapeStackAlloc, "stackalloc int[1]").WithArguments("System.Span<int>").WithLocation(13, 14),
// (19,13): error CS8353: A result of a stackalloc expression of type 'Span<int>' cannot be used in this context because it may be exposed outside of the containing method
// stackalloc int[1]; // 2
Diagnostic(ErrorCode.ERR_EscapeStackAlloc, "stackalloc int[1]").WithArguments("System.Span<int>").WithLocation(19, 13));
}

[Theory]
Expand Down
Loading

0 comments on commit 76cd2f1

Please sign in to comment.