Skip to content

Commit 3eae1b8

Browse files
RikkiGibsonRikkiGibson
authored
Ensure that locals at the top level of a constructor have the same safe-context as parameters (#80807)
Closes #80745
1 parent 9cdd3c9 commit 3eae1b8

File tree

3 files changed

+407
-11
lines changed

3 files changed

+407
-11
lines changed

‎src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ internal static void Analyze(CSharpCompilation compilation, MethodSymbol symbol,
2121
var visitor = new RefSafetyAnalysis(
2222
compilation,
2323
symbol,
24+
node,
2425
inUnsafeRegion: InUnsafeMethod(symbol),
2526
useUpdatedEscapeRules: symbol.ContainingModule.UseUpdatedEscapeRules,
2627
diagnostics);
@@ -57,6 +58,7 @@ private static bool InUnsafeMethod(Symbol symbol)
5758

5859
private readonly CSharpCompilation _compilation;
5960
private readonly MethodSymbol _symbol;
61+
private readonly BoundNode _rootNode;
6062
private readonly bool _useUpdatedEscapeRules;
6163
private readonly BindingDiagnosticBag _diagnostics;
6264
private bool _inUnsafeRegion;
@@ -72,31 +74,35 @@ private static bool InUnsafeMethod(Symbol symbol)
7274
private RefSafetyAnalysis(
7375
CSharpCompilation compilation,
7476
MethodSymbol symbol,
77+
BoundNode rootNode,
7578
bool inUnsafeRegion,
7679
bool useUpdatedEscapeRules,
7780
BindingDiagnosticBag diagnostics)
7881
{
7982
_compilation = compilation;
8083
_symbol = symbol;
84+
_rootNode = rootNode;
8185
_useUpdatedEscapeRules = useUpdatedEscapeRules;
8286
_diagnostics = diagnostics;
8387
_inUnsafeRegion = inUnsafeRegion;
84-
// _localScopeDepth is incremented at each block in the method, including the
85-
// outermost. To ensure that locals in the outermost block are considered at
86-
// the same depth as parameters, _localScopeDepth is initialized to one less.
87-
_localScopeDepth = SafeContext.CurrentMethod.Wider();
88+
_localScopeDepth = SafeContext.CurrentMethod;
8889
}
8990

9091
private ref struct LocalScope
9192
{
9293
private readonly RefSafetyAnalysis _analysis;
9394
private readonly ImmutableArray<LocalSymbol> _locals;
95+
private readonly bool _adjustDepth;
9496

95-
public LocalScope(RefSafetyAnalysis analysis, ImmutableArray<LocalSymbol> locals)
97+
/// <param name="adjustDepth">When true, narrows <see cref="_localScopeDepth"/> when the instance is created, and widens it when the instance is disposed.</param>
98+
public LocalScope(RefSafetyAnalysis analysis, ImmutableArray<LocalSymbol> locals, bool adjustDepth = true)
9699
{
97100
_analysis = analysis;
98101
_locals = locals;
99-
_analysis._localScopeDepth = _analysis._localScopeDepth.Narrower();
102+
_adjustDepth = adjustDepth;
103+
if (adjustDepth)
104+
_analysis._localScopeDepth = _analysis._localScopeDepth.Narrower();
105+
100106
foreach (var local in locals)
101107
{
102108
_analysis.AddLocalScopes(local, refEscapeScope: _analysis._localScopeDepth, valEscapeScope: SafeContext.CallingMethod);
@@ -109,7 +115,9 @@ public void Dispose()
109115
{
110116
_analysis.RemoveLocalScopes(local);
111117
}
112-
_analysis._localScopeDepth = _analysis._localScopeDepth.Wider();
118+
119+
if (_adjustDepth)
120+
_analysis._localScopeDepth = _analysis._localScopeDepth.Wider();
113121
}
114122
}
115123

@@ -290,7 +298,18 @@ private bool ContainsPlaceholderScope(BoundValuePlaceholderBase placeholder)
290298
public override BoundNode? VisitBlock(BoundBlock node)
291299
{
292300
using var _1 = new UnsafeRegion(this, _inUnsafeRegion || node.HasUnsafeModifier);
293-
using var _2 = new LocalScope(this, node.Locals);
301+
302+
// Do not increase the depth if this is the top-level block of a method body.
303+
// This is needed to ensure that top-level locals have the same lifetime as by-value parameters, for example.
304+
bool adjustDepth = _rootNode switch
305+
{
306+
BoundConstructorMethodBody constructorBody => constructorBody.BlockBody != node && constructorBody.ExpressionBody != node,
307+
BoundNonConstructorMethodBody methodBody => methodBody.BlockBody != node && methodBody.ExpressionBody != node,
308+
BoundLambda lambda => lambda.Body != node,
309+
BoundLocalFunctionStatement localFunction => localFunction.Body != node,
310+
_ => true,
311+
};
312+
using var _2 = new LocalScope(this, node.Locals, adjustDepth);
294313
return base.VisitBlock(node);
295314
}
296315

@@ -350,7 +369,7 @@ private void AssertVisited(BoundExpression expr)
350369
public override BoundNode? VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
351370
{
352371
var localFunction = (LocalFunctionSymbol)node.Symbol;
353-
var analysis = new RefSafetyAnalysis(_compilation, localFunction, _inUnsafeRegion || localFunction.IsUnsafe, _useUpdatedEscapeRules, _diagnostics);
372+
var analysis = new RefSafetyAnalysis(_compilation, localFunction, node, _inUnsafeRegion || localFunction.IsUnsafe, _useUpdatedEscapeRules, _diagnostics);
354373
analysis.Visit(node.BlockBody);
355374
analysis.Visit(node.ExpressionBody);
356375
return null;
@@ -359,14 +378,15 @@ private void AssertVisited(BoundExpression expr)
359378
public override BoundNode? VisitLambda(BoundLambda node)
360379
{
361380
var lambda = node.Symbol;
362-
var analysis = new RefSafetyAnalysis(_compilation, lambda, _inUnsafeRegion, _useUpdatedEscapeRules, _diagnostics);
381+
var analysis = new RefSafetyAnalysis(_compilation, lambda, node, _inUnsafeRegion, _useUpdatedEscapeRules, _diagnostics);
363382
analysis.Visit(node.Body);
364383
return null;
365384
}
366385

367386
public override BoundNode? VisitConstructorMethodBody(BoundConstructorMethodBody node)
368387
{
369-
using var _ = new LocalScope(this, node.Locals);
388+
// Variables in a constructor initializer like `public MyType(int x) : this(M(out int y))` have the same scope as the parameters.
389+
using var _ = new LocalScope(this, node.Locals, adjustDepth: false);
370390
return base.VisitConstructorMethodBody(node);
371391
}
372392

‎src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26561,6 +26561,143 @@ .locals init (System.ReadOnlySpan<T> V_0, //r1
2656126561
""");
2656226562
}
2656326563

26564+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80745")]
26565+
public void SpanAssignment_ScopedParameter_Constructor_Span()
26566+
{
26567+
string source = """
26568+
using System;
26569+
26570+
class C
26571+
{
26572+
public C(scoped ReadOnlySpan<int> items)
26573+
{
26574+
int x = 0;
26575+
items = new ReadOnlySpan<int>(ref x);
26576+
}
26577+
26578+
void M(scoped ReadOnlySpan<int> items)
26579+
{
26580+
int x = 0;
26581+
items = new ReadOnlySpan<int>(ref x);
26582+
}
26583+
}
26584+
""";
26585+
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net80);
26586+
comp.VerifyEmitDiagnostics();
26587+
}
26588+
26589+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80745")]
26590+
public void SpanAssignment_ScopedParameter_Constructor_CollectionExpression()
26591+
{
26592+
string source = """
26593+
using System;
26594+
26595+
class C
26596+
{
26597+
public C(scoped ReadOnlySpan<int> items)
26598+
{
26599+
int x = 0;
26600+
items = [x];
26601+
}
26602+
26603+
void M(scoped ReadOnlySpan<int> items)
26604+
{
26605+
int x = 0;
26606+
items = [x];
26607+
}
26608+
}
26609+
""";
26610+
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net80);
26611+
comp.VerifyEmitDiagnostics();
26612+
}
26613+
26614+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80745")]
26615+
public void SpanAssignment_ScopedParameter_PrimaryConstructor_Span()
26616+
{
26617+
// Each field initializer can be considered its own local scope as variables declared within it are not visible to the other initializers.
26618+
string source = """
26619+
using System;
26620+
26621+
class C(scoped ReadOnlySpan<int> items) // 1
26622+
{
26623+
private int x = M0(M1(out int x1), items = new ReadOnlySpan<int>(ref x1)); // 2, 3
26624+
private int y = x1; // 4
26625+
26626+
static int M0(int x0, ReadOnlySpan<int> items) => items[0];
26627+
static int M1(out int x1) { return x1 = 0; }
26628+
}
26629+
""";
26630+
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net80);
26631+
comp.VerifyEmitDiagnostics(
26632+
// (3,34): warning CS9113: Parameter 'items' is unread.
26633+
// class C(scoped ReadOnlySpan<int> items) // 1
26634+
Diagnostic(ErrorCode.WRN_UnreadPrimaryConstructorParameter, "items").WithArguments("items").WithLocation(3, 34),
26635+
// (5,48): error CS8347: Cannot use a result of 'ReadOnlySpan<int>.ReadOnlySpan(ref readonly int)' in this context because it may expose variables referenced by parameter 'reference' outside of their declaration scope
26636+
// private int x = M0(M1(out int x1), items = new ReadOnlySpan<int>(ref x1)); // 2, 3
26637+
Diagnostic(ErrorCode.ERR_EscapeCall, "new ReadOnlySpan<int>(ref x1)").WithArguments("System.ReadOnlySpan<int>.ReadOnlySpan(ref readonly int)", "reference").WithLocation(5, 48),
26638+
// (5,74): error CS8352: Cannot use variable 'x1' in this context because it may expose referenced variables outside of their declaration scope
26639+
// private int x = M0(M1(out int x1), items = new ReadOnlySpan<int>(ref x1)); // 2, 3
26640+
Diagnostic(ErrorCode.ERR_EscapeVariable, "x1").WithArguments("x1").WithLocation(5, 74),
26641+
// (6,21): error CS0103: The name 'x1' does not exist in the current context
26642+
// private int y = x1; // 4
26643+
Diagnostic(ErrorCode.ERR_NameNotInContext, "x1").WithArguments("x1").WithLocation(6, 21));
26644+
}
26645+
26646+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80745")]
26647+
public void SpanAssignment_StaticFieldInitializer()
26648+
{
26649+
// Each field initializer can be considered its own local scope as variables declared within it are not visible to the other initializers.
26650+
string source = """
26651+
using System;
26652+
26653+
class C
26654+
{
26655+
private static int x = M0(M1(out ReadOnlySpan<int> items, out int x1), items = new ReadOnlySpan<int>(ref x1)); // 1, 2
26656+
private static int y = items[0]; // 3
26657+
26658+
static int M0(int x0, ReadOnlySpan<int> items) => items[0];
26659+
static int M1(out ReadOnlySpan<int> span, out int x1) { span = default; return x1 = 0; }
26660+
}
26661+
""";
26662+
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net80);
26663+
comp.VerifyEmitDiagnostics(
26664+
// (5,84): error CS8347: Cannot use a result of 'ReadOnlySpan<int>.ReadOnlySpan(ref readonly int)' in this context because it may expose variables referenced by parameter 'reference' outside of their declaration scope
26665+
// private static int x = M0(M1(out ReadOnlySpan<int> items, out int x1), items = new ReadOnlySpan<int>(ref x1)); // 1, 2
26666+
Diagnostic(ErrorCode.ERR_EscapeCall, "new ReadOnlySpan<int>(ref x1)").WithArguments("System.ReadOnlySpan<int>.ReadOnlySpan(ref readonly int)", "reference").WithLocation(5, 84),
26667+
// (5,110): error CS8168: Cannot return local 'x1' by reference because it is not a ref local
26668+
// private static int x = M0(M1(out ReadOnlySpan<int> items, out int x1), items = new ReadOnlySpan<int>(ref x1)); // 1, 2
26669+
Diagnostic(ErrorCode.ERR_RefReturnLocal, "x1").WithArguments("x1").WithLocation(5, 110),
26670+
// (6,28): error CS0103: The name 'items' does not exist in the current context
26671+
// private static int y = items[0]; // 3
26672+
Diagnostic(ErrorCode.ERR_NameNotInContext, "items").WithArguments("items").WithLocation(6, 28));
26673+
}
26674+
26675+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/80745")]
26676+
public void SpanAssignment_ScopedParameter_PrimaryConstructor_CollectionExpression()
26677+
{
26678+
// '[M1()]' cannot be assigned to 'items' because its backing storage lives in a narrower scope than 'items'.
26679+
string source = """
26680+
using System;
26681+
26682+
class C(scoped ReadOnlySpan<int> items)
26683+
{
26684+
private int x = M0(items = [M1()]);
26685+
26686+
static int M0(ReadOnlySpan<int> x) => x[0];
26687+
static int M1() => 0;
26688+
}
26689+
""";
26690+
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net80);
26691+
comp.VerifyEmitDiagnostics(
26692+
// (3,34): warning CS9113: Parameter 'items' is unread.
26693+
// class C(scoped ReadOnlySpan<int> items)
26694+
Diagnostic(ErrorCode.WRN_UnreadPrimaryConstructorParameter, "items").WithArguments("items").WithLocation(3, 34),
26695+
// (5,32): error CS9203: A collection expression of type 'ReadOnlySpan<int>' cannot be used in this context because it may be exposed outside of the current scope.
26696+
// private int x = M0(items = [M1()]);
26697+
Diagnostic(ErrorCode.ERR_CollectionExpressionEscape, "[M1()]").WithArguments("System.ReadOnlySpan<int>").WithLocation(5, 32)
26698+
);
26699+
}
26700+
2656426701
[Fact]
2656526702
public void SpanAssignment_WithUsingDeclaration()
2656626703
{

0 commit comments

Comments
 (0)