From 11d73ae23998c281396f517c3bfba187ae7e980f Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 8 Apr 2024 15:54:23 +0200 Subject: [PATCH 1/5] Add a test --- .../CSharp/Portable/Binder/RefSafetyAnalysis.cs | 2 +- .../Test/Semantic/Semantics/RefEscapingTests.cs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs b/src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs index 2029d8d5aa669..8d4310758fc17 100644 --- a/src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs +++ b/src/Compilers/CSharp/Portable/Binder/RefSafetyAnalysis.cs @@ -283,7 +283,7 @@ private void AssertVisited(BoundExpression expr) } else if (_visited is { } && _visited.Count <= MaxTrackVisited) { - Debug.Assert(_visited.Contains(expr)); + Debug.Assert(_visited.Contains(expr), $"Expected {expr} `{expr.Syntax}` to be visited."); } } #endif diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index 6da08b9784005..8565dc09421cd 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -8531,5 +8531,16 @@ static R F2() // return new R(1) | new R(2); Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "1").WithLocation(18, 22)); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72873")] + public void Utf8Addition() + { + var code = """ + using System; + ReadOnlySpan x = "Hello"u8 + " "u8 + "World!"u8; + Console.WriteLine(x.Length); + """; + CreateCompilation(code, targetFramework: TargetFramework.Net70).VerifyDiagnostics(); + } } } From a6261d161ba6b9954a6e3ed5515574270738515c Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 8 Apr 2024 16:06:18 +0200 Subject: [PATCH 2/5] Avoid asserting unvisited nested binary operators --- .../Portable/Binder/Binder.ValueChecks.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 002b0a25e6cb1..22dd3f59fca8b 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -3689,10 +3689,17 @@ internal uint GetBroadestValEscape(BoundTupleExpression expr, uint scopeOfTheCon /// /// NOTE: unless the type of expression is ref-like, the result is Binder.ExternalScope since ordinary values can always be returned from methods. /// - internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpression) + internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpression +#if DEBUG + , bool assertVisited = true +#endif + ) { #if DEBUG - AssertVisited(expr); + if (assertVisited) + { + AssertVisited(expr); + } #endif // cannot infer anything from errors @@ -4069,8 +4076,21 @@ internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpres isRefEscape: false); } - return Math.Max(GetValEscape(binary.Left, scopeOfTheContainingExpression), - GetValEscape(binary.Right, scopeOfTheContainingExpression)); +#if DEBUG + // Nested binary operators on the left side are not visited by BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator. + var assertNestedVisited = binary.Left is not BoundBinaryOperator; +#endif + + return Math.Max(GetValEscape(binary.Left, scopeOfTheContainingExpression +#if DEBUG + , assertNestedVisited +#endif + ), + GetValEscape(binary.Right, scopeOfTheContainingExpression +#if DEBUG + , assertNestedVisited +#endif + )); case BoundKind.RangeExpression: var range = (BoundRangeExpression)expr; From a2ffae674a3a23743a9cedaef28826a4c8d14e09 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 15 Apr 2024 13:32:26 +0200 Subject: [PATCH 3/5] Revert "Avoid asserting unvisited nested binary operators" This reverts commit a6261d161ba6b9954a6e3ed5515574270738515c. --- .../Portable/Binder/Binder.ValueChecks.cs | 28 +++---------------- 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 22dd3f59fca8b..002b0a25e6cb1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -3689,17 +3689,10 @@ internal uint GetBroadestValEscape(BoundTupleExpression expr, uint scopeOfTheCon /// /// NOTE: unless the type of expression is ref-like, the result is Binder.ExternalScope since ordinary values can always be returned from methods. /// - internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpression -#if DEBUG - , bool assertVisited = true -#endif - ) + internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpression) { #if DEBUG - if (assertVisited) - { - AssertVisited(expr); - } + AssertVisited(expr); #endif // cannot infer anything from errors @@ -4076,21 +4069,8 @@ internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpres isRefEscape: false); } -#if DEBUG - // Nested binary operators on the left side are not visited by BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator. - var assertNestedVisited = binary.Left is not BoundBinaryOperator; -#endif - - return Math.Max(GetValEscape(binary.Left, scopeOfTheContainingExpression -#if DEBUG - , assertNestedVisited -#endif - ), - GetValEscape(binary.Right, scopeOfTheContainingExpression -#if DEBUG - , assertNestedVisited -#endif - )); + return Math.Max(GetValEscape(binary.Left, scopeOfTheContainingExpression), + GetValEscape(binary.Right, scopeOfTheContainingExpression)); case BoundKind.RangeExpression: var range = (BoundRangeExpression)expr; From 10d62835577aac52c146138ca121db71b0384cec Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Mon, 15 Apr 2024 13:30:21 +0200 Subject: [PATCH 4/5] Test nested binary operator --- .../Semantic/Semantics/RefEscapingTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index 8565dc09421cd..0cd3ee133280d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -7871,6 +7871,37 @@ ref struct S Diagnostic(ErrorCode.ERR_EscapeVariable, "s").WithArguments("s").WithLocation(47, 16)); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71773")] + public void UserDefinedBinaryOperator_RefStruct_Nested() + { + var source = """ + class C + { + S M() + { + S s; + s = default(S) + 100 + 200; + return s; + } + } + + ref struct S + { + public static S operator+(S y, in int x) => throw null; + } + """; + CreateCompilation(source).VerifyDiagnostics( + // (6,13): error CS8347: Cannot use a result of 'S.operator +(S, in int)' in this context because it may expose variables referenced by parameter 'x' outside of their declaration scope + // s = default(S) + 100 + 200; + Diagnostic(ErrorCode.ERR_EscapeCall, "default(S) + 100").WithArguments("S.operator +(S, in int)", "x").WithLocation(6, 13), + // (6,13): error CS8347: Cannot use a result of 'S.operator +(S, in int)' in this context because it may expose variables referenced by parameter 'y' outside of their declaration scope + // s = default(S) + 100 + 200; + Diagnostic(ErrorCode.ERR_EscapeCall, "default(S) + 100 + 200").WithArguments("S.operator +(S, in int)", "y").WithLocation(6, 13), + // (6,26): error CS8156: An expression cannot be used in this context because it may not be passed or returned by reference + // s = default(S) + 100 + 200; + Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "100").WithLocation(6, 26)); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71773")] public void UserDefinedBinaryOperator_RefStruct_Scoped_Left() { From af94aa23e33560e8d0e6c32cb638841745ee00a5 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 16 Apr 2024 13:22:54 +0200 Subject: [PATCH 5/5] Add manually to visited set --- .../CSharp/Portable/Binder/Binder.ValueChecks.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 002b0a25e6cb1..98844f4c0150a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -4055,6 +4055,14 @@ internal uint GetValEscape(BoundExpression expr, uint scopeOfTheContainingExpres case BoundKind.BinaryOperator: var binary = (BoundBinaryOperator)expr; +#if DEBUG + // Nested binary operators on the left-hand side are not visited by BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator. + if (binary.Left is BoundBinaryOperator) + { + _visited.Add(binary.Left); + } +#endif + if (binary.Method is { } binaryMethod) { return GetInvocationEscapeScope( @@ -4734,6 +4742,14 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint escapeF return true; } +#if DEBUG + // Nested binary operators on the left-hand side are not visited by BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator. + if (binary.Left is BoundBinaryOperator) + { + _visited.Add(binary.Left); + } +#endif + if (binary.Method is { } binaryMethod) { return CheckInvocationEscape(