Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ref safety: replace uint scope with struct SafeContext #75647

Merged
merged 15 commits into from
Nov 8, 2024
Merged
310 changes: 143 additions & 167 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4377,13 +4377,13 @@ private void ValidateRefConditionalOperator(SyntaxNode node, BoundExpression tru
var currentScope = _localScopeDepth;

// val-escape must agree on both branches.
uint whenTrueEscape = GetValEscape(trueExpr, currentScope);
uint whenFalseEscape = GetValEscape(falseExpr, currentScope);
Lifetime whenTrueEscape = GetValEscape(trueExpr, currentScope);
Lifetime whenFalseEscape = GetValEscape(falseExpr, currentScope);

if (whenTrueEscape != whenFalseEscape)
if (!whenTrueEscape.Equals(whenFalseEscape))
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
// ask the one with narrower escape, for the wider - hopefully the errors will make the violation easier to fix.
if (whenTrueEscape < whenFalseEscape)
if (!whenFalseEscape.IsConvertibleTo(whenTrueEscape))
CheckValEscape(falseExpr.Syntax, falseExpr, currentScope, whenTrueEscape, checkingReceiver: false, diagnostics: diagnostics);
else
CheckValEscape(trueExpr.Syntax, trueExpr, currentScope, whenFalseEscape, checkingReceiver: false, diagnostics: diagnostics);
Expand Down
17 changes: 9 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1549,12 +1549,12 @@ private void ValidateAssignment(

var leftEscape = GetRefEscape(op1, _localScopeDepth);
var rightEscape = GetRefEscape(op2, _localScopeDepth);
if (leftEscape < rightEscape)
if (!rightEscape.IsConvertibleTo(leftEscape))
{
var errorCode = (rightEscape, _inUnsafeRegion) switch
{
(ReturnOnlyScope, false) => ErrorCode.ERR_RefAssignReturnOnly,
(ReturnOnlyScope, true) => ErrorCode.WRN_RefAssignReturnOnly,
({ IsReturnOnly: true }, false) => ErrorCode.ERR_RefAssignReturnOnly,
({ IsReturnOnly: true }, true) => ErrorCode.WRN_RefAssignReturnOnly,
(_, false) => ErrorCode.ERR_RefAssignNarrower,
(_, true) => ErrorCode.WRN_RefAssignNarrower
};
Expand All @@ -1570,12 +1570,13 @@ private void ValidateAssignment(
leftEscape = GetValEscape(op1, _localScopeDepth);
rightEscape = GetValEscape(op2, _localScopeDepth);

Debug.Assert(leftEscape == rightEscape || op1.Type.IsRefLikeOrAllowsRefLikeType());
Debug.Assert(leftEscape.Equals(rightEscape) || op1.Type.IsRefLikeOrAllowsRefLikeType());

// We only check if the safe-to-escape of e2 is wider than the safe-to-escape of e1 here,
// we don't check for equality. The case where the safe-to-escape of e2 is narrower than
// e1 is handled in the if (op1.Type.IsRefLikeType) { ... } block later.
if (leftEscape > rightEscape)
// We only check if the left lifetime is convertible to the right here
// in order to give a more useful diagnostic.
// Later on we check if right lifetime is convertible to left,
// which effectively means these lifetimes must be equal.
if (!leftEscape.IsConvertibleTo(rightEscape))
{
Debug.Assert(op1.Kind != BoundKind.Parameter); // If the assert fails, add a corresponding test.

Expand Down
131 changes: 131 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Lifetime.cs
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
/// <summary>
/// A representation of the program region in which the *referent* of a `ref` is *live*.
/// Limited to what is expressible in C#.
/// </summary>
/// <remarks>
/// - A *referent* is the variable being referenced by a `ref`.
/// - Informally, a variable is *live* if it has storage allocated for it (either on heap or stack).
/// - In this design, all lifetimes have a known relationship to all other lifetimes.
/// </remarks>
internal readonly struct Lifetime
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
private const uint CallingMethodRaw = 0;
private const uint ReturnOnlyRaw = 1;
private const uint CurrentMethodRaw = 2;

/// <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.
/// 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>
private readonly uint _value;
private Lifetime(uint value) => _value = value;

/// <summary>
/// The "calling method" scope that is outside of the containing method/lambda.
/// If something can escape to this scope, it can escape to any scope in a given method through a ref parameter or return.
/// </summary>
public static Lifetime CallingMethod => new Lifetime(CallingMethodRaw);
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The "return-only" scope that is outside of the containing method/lambda.
/// If something can escape to this scope, it can escape to any scope in a given method or can be returned, but it can't escape through a ref parameter.
/// </summary>
public static Lifetime ReturnOnly => new Lifetime(ReturnOnlyRaw);

/// <summary>
/// The "current method" scope that is just inside the containing method/lambda.
/// If something can escape to this scope, it can escape to any scope in a given method, but cannot be returned.
/// </summary>
public static Lifetime CurrentMethod => new Lifetime(CurrentMethodRaw);

/// <summary>
/// Gets a lifetime which is "empty". i.e. which refers to a variable whose storage is never allocated.
/// </summary>
public static Lifetime Empty => new Lifetime(uint.MaxValue);

/// <summary>
/// Gets a lifetime which is narrower than the given lifetime.
/// Used to "enter" a nested local scope.
/// </summary>
public Lifetime Narrower()
{
var result = new Lifetime(_value + 1);
// Narrower() operator should always result in a local lifetime
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
Debug.Assert(!result.IsReturnable);
return result;
}

/// <summary>
/// Gets a lifetime which is wider than the given lifetime.
/// Used to "exit" a nested local scope.
/// </summary>
public Lifetime Wider()
{
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
// Wider() operator should always start from a local lifetime
Debug.Assert(!IsReturnable);
return new Lifetime(_value - 1);
}

public bool IsCallingMethod => _value == CallingMethodRaw;
public bool IsReturnOnly => _value == ReturnOnlyRaw;
public bool IsReturnable => _value is CallingMethodRaw or ReturnOnlyRaw;

/// <summary>Returns true if a 'ref' with this lifetime can be converted to the 'other' lifetime. Otherwise, returns false.</summary>
/// <remarks>Generally, a wider lifetime is convertible to a narrower lifetime.</remarks>
public bool IsConvertibleTo(Lifetime other)
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
=> this._value <= other._value;

/// <summary>
/// Returns the narrower of two lifetimes.
/// </summary>
/// <remarks>
/// In other words, this method returns the widest lifetime which 'this' and 'other' are both convertible to.
/// If in future we added the concept of unrelated lifetimes (e.g. to implement 'ref scoped'), this method would perhaps return a Nullable,
/// for the case that no lifetime exists which both input lifetimes are convertible to.
/// </remarks>
public Lifetime Intersect(Lifetime other)
=> this.IsConvertibleTo(other) ? other : this;

/// <summary>
/// Returns the wider of two lifetimes.
/// </summary>
/// <remarks>In other words, this method returns the narrowest lifetime which can be converted to both 'this' and 'other'.</remarks>
public Lifetime Union(Lifetime other)
=> this.IsConvertibleTo(other) ? this : other;

/// <summary>Returns true if this lifetime is the same as 'other' (i.e. for invariant nested conversion).</summary>
public bool Equals(Lifetime other)
=> this._value == other._value;

public override bool Equals(object? obj)
=> obj is Lifetime other && this.Equals(other);

public override int GetHashCode()
=> unchecked((int)_value);

public static bool operator ==(Lifetime lhs, Lifetime rhs)
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
=> lhs._value == rhs._value;

public static bool operator !=(Lifetime lhs, Lifetime rhs)
=> lhs._value != rhs._value;

public override string ToString()
=> _value switch
{
CallingMethodRaw => "Lifetime<CallingMethod>",
ReturnOnlyRaw => "Lifetime<ReturnOnly>",
CurrentMethodRaw => "Lifetime<CurrentMethod>",
_ => $"Lifetime<{_value}>"
};
}
}
Loading