Skip to content

Commit

Permalink
Ref safety: replace uint scope with struct SafeContext (#75647)
Browse files Browse the repository at this point in the history
Co-authored-by: Jan Jones <jan.jones.cz@gmail.com>
Co-authored-by: Jared Parsons <jared@paranoidcoding.org>
  • Loading branch information
3 people authored Nov 8, 2024
1 parent 82aedf1 commit 46909e9
Show file tree
Hide file tree
Showing 7 changed files with 347 additions and 236 deletions.
310 changes: 143 additions & 167 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

Large diffs are not rendered by default.

6 changes: 3 additions & 3 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);
SafeContext whenTrueEscape = GetValEscape(trueExpr, currentScope);
SafeContext whenFalseEscape = GetValEscape(falseExpr, currentScope);

if (whenTrueEscape != whenFalseEscape)
{
// 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 @@ -1551,12 +1551,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 @@ -1572,12 +1572,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 SafeContext is convertible to the right here
// in order to give a more useful diagnostic.
// Later on we check if right SafeContext is convertible to left,
// which effectively means these SafeContexts must be equal.
if (!leftEscape.IsConvertibleTo(rightEscape))
{
Debug.Assert(op1.Kind != BoundKind.Parameter); // If the assert fails, add a corresponding test.

Expand Down
Loading

0 comments on commit 46909e9

Please sign in to comment.