Skip to content

Commit

Permalink
Fix UnscopedRef handling in indexers
Browse files Browse the repository at this point in the history 
The object initializer logic wasn't completely handling the lifetime of
the `in` part of parameters. It was considering it escaping even if it
wasn't marked as `[UnscopedRef]`

closes #66056
  • Loading branch information
@jaredpar
jaredpar committed Jul 15, 2024
1 parent e2bfb31 commit 33411cd
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 96 deletions.
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection.Metadata.Ecma335;
using Microsoft.CodeAnalysis.CSharp.CodeGen;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -4624,6 +4625,13 @@ uint getIndexerEscape(
uint receiverEscapeScope = CallingMethodScope;
foreach (var escapeValue in escapeValues)
{
// This is a call to an indexer so the ref escape scope can only impact the escape value if it
// can be assigned to `this`. Return Only can't do this.
if (escapeValue.IsRefEscape && escapeValue.EscapeLevel != EscapeLevel.CallingMethod)
{
continue;
}

uint escapeScope = escapeValue.IsRefEscape
? GetRefEscape(escapeValue.Argument, scopeOfTheContainingExpression)
: GetValEscape(escapeValue.Argument, scopeOfTheContainingExpression);
Expand Down
241 changes: 145 additions & 96 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3369,6 +3369,89 @@ static void Test()
);
}

[Fact]
public void RefLikeEscapeMixingObjectInitializer4()
{
var tree = SyntaxFactory.ParseSyntaxTree("""
using System;

public ref struct S1
{
public S1(ref Span<int> span) { }
int this[in Span<int> span] { get => 0; set {} }

static void Test()
{
Span<int> stackSpan = stackalloc int[10];
Span<int> heapSpan = default;

var local1 = new S1(ref heapSpan) { [heapSpan] = 0 };
var local2 = new S1(ref heapSpan) { [stackSpan] = 0 }; // 1
var local3 = new S1(ref stackSpan) { [heapSpan] = 0 };
var local4 = new S1(ref stackSpan) { [stackSpan] = 0 };
}
}

public ref struct S2
{
public S2(scoped ref Span<int> span) { }
int this[Span<int> span] { get => 0; set {} }

static void Test()
{
Span<int> stackSpan = stackalloc int[10];
Span<int> heapSpan = default;

var local1 = new S2(ref heapSpan) { [heapSpan] = 0 };
var local2 = new S2(ref heapSpan) { [stackSpan] = 0 };
var local3 = new S2(ref stackSpan) { [heapSpan] = 0 };
var local4 = new S2(ref stackSpan) { [stackSpan] = 0 };
}
}

public ref struct S3
{
public S3(ref Span<int> span) { }
int this[scoped Span<int> span] { get => 0; set {} }

static void Test()
{
Span<int> stackSpan = stackalloc int[10];
Span<int> heapSpan = default;

var local1 = new S3(ref heapSpan) { [heapSpan] = 0 };
var local2 = new S3(ref heapSpan) { [stackSpan] = 0 };
var local3 = new S3(ref stackSpan) { [heapSpan] = 0 };
var local4 = new S3(ref stackSpan) { [stackSpan] = 0 };
}
}

public struct S5
{
public S5(Span<int> span) { }
Span<int> this[int index] { get => default; set { } }

static void Test()
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;

S5 local;
local = new S5(stackSpan) { [0] = stackSpan };
local = new S5(stackSpan) { [0] = heapSpan };
local = new S5(heapSpan) { [0] = stackSpan };
local = new S5(heapSpan) { [0] = heapSpan };
}
}
""", TestOptions.Regular.WithLanguageVersion(LanguageVersion.CSharp11));
var comp = CreateCompilationWithSpan(tree, TestOptions.UnsafeDebugDll);
comp.VerifyEmitDiagnostics(
// (14,33): error CS8350: This combination of arguments to 'S1.S1(ref Span<int>)' is disallowed because it may expose variables referenced by parameter 'span' outside of their declaration scope
// var local2 = new S1(ref heapSpan) { [stackSpan] = 0 }; // 1
Diagnostic(ErrorCode.ERR_CallArgMixing, "heapSpan").WithArguments("S1.S1(ref System.Span<int>)", "span").WithLocation(14, 33)
);
}

[Theory]
[InlineData(LanguageVersion.CSharp10)]
[InlineData(LanguageVersion.CSharp11)]
Expand Down Expand Up @@ -3814,135 +3897,101 @@ static void Test()
[Fact]
public void ObjectInitializer_Constructor_Escape()
{
var tree = SyntaxFactory.ParseSyntaxTree("""
var code = """
using System;
using System.Diagnostics.CodeAnalysis;

public ref struct S1
{
public S1(Span<int> span) { }
Span<int> this[int index] { get => default; set { } }
public S1() { }
string this[int x] { get => default; set { } }

static void Test()
static void Test(int x)
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;

int y = 0;
S1 local;
local = new S1(stackSpan) { [0] = stackSpan }; // 1
local = new S1(stackSpan) { [0] = heapSpan }; // 2
local = new S1(heapSpan) { [0] = stackSpan }; // 3
local = new S1(heapSpan) { [0] = heapSpan };

local = new() { [x] = "" };
local = new() { [y] = "" };
local = new() { [0] = "" };
}
}

public ref struct S2
{
public S2(scoped Span<int> span) { }
Span<int> this[int index] { get => default; set { } }
public S2() { }
string this[in int x] { get => default; set { } }

static void Test()
static void Test(int x)
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;

int y = 0;
S2 local;
local = new S2(stackSpan) { [0] = stackSpan }; // 4
local = new S2(stackSpan) { [0] = heapSpan };
local = new S2(heapSpan) { [0] = stackSpan }; // 5
local = new S2(heapSpan) { [0] = heapSpan };

local = new() { [in x] = "" };
local = new() { [in y] = "" };
local = new() { [0] = "" };
}
}

public ref struct S3
{
public S3(Span<int> span) { }
Span<int> this[int index] { get => default; readonly set { } }
public S3() { }
string this[[UnscopedRef] in int x] { get => default; set { } }

static void Test()
static void Test(int x)
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;

int y = 0;
S3 local;
local = new S3(stackSpan) { [0] = stackSpan }; // 6
local = new S3(stackSpan) { [0] = heapSpan }; // 7
local = new S3(heapSpan) { [0] = stackSpan };
local = new S3(heapSpan) { [0] = heapSpan };

local = new() { [in x] = "" }; // 1
local = new() { [in y] = "" }; // 2
local = new() { [0] = "" }; // 3
}
}
""";

public ref struct S4
{
public S4(scoped Span<int> span) { }
Span<int> this[int index] { get => default; readonly set { } }

static void Test()
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;
var comp = CreateCompilationWithSpan([code, UnscopedRefAttributeDefinition], TestOptions.UnsafeDebugDll, TestOptions.Regular11);
comp.VerifyEmitDiagnostics(
// (46,23): error CS8352: Cannot use variable '[in x] = ""' in this context because it may expose referenced variables outside of their declaration scope
// local = new() { [in x] = "" }; // 1
Diagnostic(ErrorCode.ERR_EscapeVariable, @"{ [in x] = """" }").WithArguments(@"[in x] = """"").WithLocation(46, 23),
// (47,23): error CS8352: Cannot use variable '[in y] = ""' in this context because it may expose referenced variables outside of their declaration scope
// local = new() { [in y] = "" }; // 2
Diagnostic(ErrorCode.ERR_EscapeVariable, @"{ [in y] = """" }").WithArguments(@"[in y] = """"").WithLocation(47, 23),
// (48,23): error CS8352: Cannot use variable '[0] = ""' in this context because it may expose referenced variables outside of their declaration scope
// local = new() { [0] = "" }; // 3
Diagnostic(ErrorCode.ERR_EscapeVariable, @"{ [0] = """" }").WithArguments(@"[0] = """"").WithLocation(48, 23)
);
}

S4 local;
local = new S4(stackSpan) { [0] = stackSpan };
local = new S4(stackSpan) { [0] = heapSpan };
local = new S4(heapSpan) { [0] = stackSpan };
local = new S4(heapSpan) { [0] = heapSpan };
}
}
[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/66056")]
public void ObjectInitializer_Indexer_Unscoped()
{
var code = """
using System;
using System.Diagnostics.CodeAnalysis;

public struct S5
public ref struct S1
{
public S5(Span<int> span) { }
Span<int> this[int index] { get => default; set { } }
public S1() { }
string this[[UnscopedRef] in int x] { get => default; set { } }

static void Test()
{
Span<int> stackSpan = stackalloc int[] { 13 };
Span<int> heapSpan = default;

S5 local;
local = new S5(stackSpan) { [0] = stackSpan };
local = new S5(stackSpan) { [0] = heapSpan };
local = new S5(heapSpan) { [0] = stackSpan };
local = new S5(heapSpan) { [0] = heapSpan };
}
static S1 Test1(int x) => new S1() { [in x] = "" }; // 1
static S1 Test2(in int x) => new S1() { [in x] = "" };
static S1 Test3(scoped in int x) => new S1() { [in x] = "" }; // 2
}
""", TestOptions.Regular.WithLanguageVersion(LanguageVersion.CSharp11));

var comp = CreateCompilationWithSpan(tree, TestOptions.UnsafeDebugDll);
""";

var comp = CreateCompilationWithSpan([code, UnscopedRefAttributeDefinition], TestOptions.UnsafeDebugDll, TestOptions.Regular11);
comp.VerifyEmitDiagnostics(
// (14,17): error CS8347: Cannot use a result of 'S1.S1(Span<int>)' in this context because it may expose variables referenced by parameter 'span' outside of their declaration scope
// local = new S1(stackSpan) { [0] = stackSpan }; // 1
Diagnostic(ErrorCode.ERR_EscapeCall, "new S1(stackSpan) { [0] = stackSpan }").WithArguments("S1.S1(System.Span<int>)", "span").WithLocation(14, 17),
// (14,24): error CS8352: Cannot use variable 'stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S1(stackSpan) { [0] = stackSpan }; // 1
Diagnostic(ErrorCode.ERR_EscapeVariable, "stackSpan").WithArguments("stackSpan").WithLocation(14, 24),
// (15,17): error CS8347: Cannot use a result of 'S1.S1(Span<int>)' in this context because it may expose variables referenced by parameter 'span' outside of their declaration scope
// local = new S1(stackSpan) { [0] = heapSpan }; // 2
Diagnostic(ErrorCode.ERR_EscapeCall, "new S1(stackSpan) { [0] = heapSpan }").WithArguments("S1.S1(System.Span<int>)", "span").WithLocation(15, 17),
// (15,24): error CS8352: Cannot use variable 'stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S1(stackSpan) { [0] = heapSpan }; // 2
Diagnostic(ErrorCode.ERR_EscapeVariable, "stackSpan").WithArguments("stackSpan").WithLocation(15, 24),
// (16,34): error CS8352: Cannot use variable '[0] = stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S1(heapSpan) { [0] = stackSpan }; // 3
Diagnostic(ErrorCode.ERR_EscapeVariable, "{ [0] = stackSpan }").WithArguments("[0] = stackSpan").WithLocation(16, 34),
// (32,35): error CS8352: Cannot use variable '[0] = stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S2(stackSpan) { [0] = stackSpan }; // 4
Diagnostic(ErrorCode.ERR_EscapeVariable, "{ [0] = stackSpan }").WithArguments("[0] = stackSpan").WithLocation(32, 35),
// (34,34): error CS8352: Cannot use variable '[0] = stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S2(heapSpan) { [0] = stackSpan }; // 5
Diagnostic(ErrorCode.ERR_EscapeVariable, "{ [0] = stackSpan }").WithArguments("[0] = stackSpan").WithLocation(34, 34),
// (50,17): error CS8347: Cannot use a result of 'S3.S3(Span<int>)' in this context because it may expose variables referenced by parameter 'span' outside of their declaration scope
// local = new S3(stackSpan) { [0] = stackSpan }; // 6
Diagnostic(ErrorCode.ERR_EscapeCall, "new S3(stackSpan) { [0] = stackSpan }").WithArguments("S3.S3(System.Span<int>)", "span").WithLocation(50, 17),
// (50,24): error CS8352: Cannot use variable 'stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S3(stackSpan) { [0] = stackSpan }; // 6
Diagnostic(ErrorCode.ERR_EscapeVariable, "stackSpan").WithArguments("stackSpan").WithLocation(50, 24),
// (51,17): error CS8347: Cannot use a result of 'S3.S3(Span<int>)' in this context because it may expose variables referenced by parameter 'span' outside of their declaration scope
// local = new S3(stackSpan) { [0] = heapSpan }; // 7
Diagnostic(ErrorCode.ERR_EscapeCall, "new S3(stackSpan) { [0] = heapSpan }").WithArguments("S3.S3(System.Span<int>)", "span").WithLocation(51, 17),
// (51,24): error CS8352: Cannot use variable 'stackSpan' in this context because it may expose referenced variables outside of their declaration scope
// local = new S3(stackSpan) { [0] = heapSpan }; // 7
Diagnostic(ErrorCode.ERR_EscapeVariable, "stackSpan").WithArguments("stackSpan").WithLocation(51, 24)
// (9,40): error CS8352: Cannot use variable '[in x] = ""' in this context because it may expose referenced variables outside of their declaration scope
// static S1 Test1(int x) => new S1() { [in x] = "" }; // 1
Diagnostic(ErrorCode.ERR_EscapeVariable, @"{ [in x] = """" }").WithArguments(@"[in x] = """"").WithLocation(9, 40),
// (11,50): error CS8352: Cannot use variable '[in x] = ""' in this context because it may expose referenced variables outside of their declaration scope
// static S1 Test3(scoped in int x) => new S1() { [in x] = "" }; // 2
Diagnostic(ErrorCode.ERR_EscapeVariable, @"{ [in x] = """" }").WithArguments(@"[in x] = """"").WithLocation(11, 50)
);
}

Expand Down

0 comments on commit 33411cd

Please sign in to comment.