From 9b25fbfd273d1a69e6dd5062e1c09fcc4109f7b5 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 24 Oct 2024 10:54:10 +0200 Subject: [PATCH 1/2] Fix ref analysis of self-assignment --- .../Portable/Binder/Binder.ValueChecks.cs | 5 -- .../Semantic/Semantics/RefEscapingTests.cs | 73 +++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index aea6580634b3a..7cd80faf644bc 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -2920,11 +2920,6 @@ private bool CheckInvocationArgMixingWithUpdatedRules( var toArgEscape = GetValEscape(mixableArg.Argument, scopeOfTheContainingExpression); foreach (var (fromParameter, fromArg, escapeKind, isRefEscape) in escapeValues) { - if (mixableArg.Parameter is not null && object.ReferenceEquals(mixableArg.Parameter, fromParameter)) - { - continue; - } - // This checks to see if the EscapeValue could ever be assigned to this argument based // on comparing the EscapeLevel of both. If this could never be assigned due to // this then we don't need to consider it for MAMM analysis. diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index 937a9988ccf5d..e8b6f51bad690 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -10170,5 +10170,78 @@ public void Utf8Addition() """; CreateCompilation(code, targetFramework: TargetFramework.Net70).VerifyDiagnostics(); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75592")] + public void SelfAssignment_ReturnOnly() + { + var source = """ + ref struct S + { + int field; + ref int refField; + + static void M(ref S s) + { + s.refField = ref s.field; + } + } + """; + CreateCompilation(source, targetFramework: TargetFramework.Net70).VerifyDiagnostics( + // (8,9): error CS9079: Cannot ref-assign 's.field' to 'refField' because 's.field' can only escape the current method through a return statement. + // s.refField = ref s.field; + Diagnostic(ErrorCode.ERR_RefAssignReturnOnly, "s.refField = ref s.field").WithArguments("refField", "s.field").WithLocation(8, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75592")] + public void SelfAssignment_CallerContext() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + S s = default; + S.M(ref s); + + ref struct S + { + int field; + ref int refField; + + public static void M([UnscopedRef] ref S s) + { + s.refField = ref s.field; + } + } + """; + CreateCompilation(source, targetFramework: TargetFramework.Net70).VerifyDiagnostics( + // (4,1): error CS8350: This combination of arguments to 'S.M(ref S)' is disallowed because it may expose variables referenced by parameter 's' outside of their declaration scope + // S.M(ref s); + Diagnostic(ErrorCode.ERR_CallArgMixing, "S.M(ref s)").WithArguments("S.M(ref S)", "s").WithLocation(4, 1), + // (4,9): error CS8168: Cannot return local 's' by reference because it is not a ref local + // S.M(ref s); + Diagnostic(ErrorCode.ERR_RefReturnLocal, "s").WithArguments("s").WithLocation(4, 9)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75592")] + public void SelfAssignment_CallerContext_Scoped() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + scoped S s = default; + S.M(ref s); + + ref struct S + { + int field; + ref int refField; + + public static void M([UnscopedRef] ref S s) + { + s.refField = ref s.field; + } + } + """; + CreateCompilation(source, targetFramework: TargetFramework.Net70).VerifyDiagnostics(); + } } } From bc4d54ec5eda0a60457f549c35faf1851d806d19 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 24 Oct 2024 10:58:12 +0200 Subject: [PATCH 2/2] Update tests --- .../Test/Emit3/RefStructInterfacesTests.cs | 6 ++++++ .../Test/Semantic/Semantics/RefFieldTests.cs | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs b/src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs index 9eb341f5bb670..ee7d2e7a79947 100644 --- a/src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs @@ -26477,6 +26477,12 @@ static ref S F2(S x2) var comp = CreateCompilation(source, targetFramework: s_targetFrameworkSupportingByRefLikeGenerics); comp.VerifyEmitDiagnostics( + // (12,26): error CS8350: This combination of arguments to 'Program.F1(ref S)' is disallowed because it may expose variables referenced by parameter 'x1' outside of their declaration scope + // ref var y2 = ref F1(ref x2); + Diagnostic(ErrorCode.ERR_CallArgMixing, "F1(ref x2)").WithArguments("Program.F1(ref S)", "x1").WithLocation(12, 26), + // (12,33): error CS8166: Cannot return a parameter by reference 'x2' because it is not a ref parameter + // ref var y2 = ref F1(ref x2); + Diagnostic(ErrorCode.ERR_RefReturnParameter, "x2").WithArguments("x2").WithLocation(12, 33), // (13,20): error CS8157: Cannot return 'y2' by reference because it was initialized to a value that cannot be returned by reference // return ref y2; // 1 Diagnostic(ErrorCode.ERR_RefReturnNonreturnableLocal, "y2").WithArguments("y2").WithLocation(13, 20) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs index e1b2e1cceaae0..e69fc2bdf577b 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs @@ -9757,6 +9757,12 @@ static ref S F2(S x2) else { comp.VerifyEmitDiagnostics( + // (14,26): error CS8350: This combination of arguments to 'Program.F1(ref S)' is disallowed because it may expose variables referenced by parameter 'x1' outside of their declaration scope + // ref var y2 = ref F1(ref x2); + Diagnostic(ErrorCode.ERR_CallArgMixing, "F1(ref x2)").WithArguments("Program.F1(ref S)", "x1").WithLocation(14, 26), + // (14,33): error CS8166: Cannot return a parameter by reference 'x2' because it is not a ref parameter + // ref var y2 = ref F1(ref x2); + Diagnostic(ErrorCode.ERR_RefReturnParameter, "x2").WithArguments("x2").WithLocation(14, 33), // (15,20): error CS8157: Cannot return 'y2' by reference because it was initialized to a value that cannot be returned by reference // return ref y2; // 1 Diagnostic(ErrorCode.ERR_RefReturnNonreturnableLocal, "y2").WithArguments("y2").WithLocation(15, 20)); @@ -29523,9 +29529,15 @@ private ref struct RefStruct // (9,9): error CS8374: Cannot ref-assign 'value' to 'RefField' because 'value' has a narrower escape scope than 'RefField'. // GetReference2(in s1).RefField = ref value; // 2 Diagnostic(ErrorCode.ERR_RefAssignNarrower, "GetReference2(in s1).RefField = ref value").WithArguments("RefField", "value").WithLocation(9, 9), + // (10,9): error CS8350: This combination of arguments to 'Repro.GetReference3(out Repro.RefStruct)' is disallowed because it may expose variables referenced by parameter 'rs' outside of their declaration scope + // GetReference3(out s1).RefField = ref value; // 3 + Diagnostic(ErrorCode.ERR_CallArgMixing, "GetReference3(out s1)").WithArguments("Repro.GetReference3(out Repro.RefStruct)", "rs").WithLocation(10, 9), // (10,9): error CS8374: Cannot ref-assign 'value' to 'RefField' because 'value' has a narrower escape scope than 'RefField'. // GetReference3(out s1).RefField = ref value; // 3 Diagnostic(ErrorCode.ERR_RefAssignNarrower, "GetReference3(out s1).RefField = ref value").WithArguments("RefField", "value").WithLocation(10, 9), + // (10,27): error CS8168: Cannot return local 's1' by reference because it is not a ref local + // GetReference3(out s1).RefField = ref value; // 3 + Diagnostic(ErrorCode.ERR_RefReturnLocal, "s1").WithArguments("s1").WithLocation(10, 27), // (12,9): error CS8332: Cannot assign to a member of method 'GetReadonlyReference1' or use it as the right hand side of a ref assignment because it is a readonly variable // GetReadonlyReference1(ref s1).RefField = ref value; // 4 Diagnostic(ErrorCode.ERR_AssignReadonlyNotField2, "GetReadonlyReference1(ref s1).RefField").WithArguments("method", "GetReadonlyReference1").WithLocation(12, 9), @@ -29534,7 +29546,13 @@ private ref struct RefStruct Diagnostic(ErrorCode.ERR_AssignReadonlyNotField2, "GetReadonlyReference2(in s1).RefField").WithArguments("method", "GetReadonlyReference2").WithLocation(13, 9), // (14,9): error CS8332: Cannot assign to a member of method 'GetReadonlyReference3' or use it as the right hand side of a ref assignment because it is a readonly variable // GetReadonlyReference3(out s1).RefField = ref value; // 6 - Diagnostic(ErrorCode.ERR_AssignReadonlyNotField2, "GetReadonlyReference3(out s1).RefField").WithArguments("method", "GetReadonlyReference3").WithLocation(14, 9)); + Diagnostic(ErrorCode.ERR_AssignReadonlyNotField2, "GetReadonlyReference3(out s1).RefField").WithArguments("method", "GetReadonlyReference3").WithLocation(14, 9), + // (14,9): error CS8350: This combination of arguments to 'Repro.GetReadonlyReference3(out Repro.RefStruct)' is disallowed because it may expose variables referenced by parameter 'rs' outside of their declaration scope + // GetReadonlyReference3(out s1).RefField = ref value; // 6 + Diagnostic(ErrorCode.ERR_CallArgMixing, "GetReadonlyReference3(out s1)").WithArguments("Repro.GetReadonlyReference3(out Repro.RefStruct)", "rs").WithLocation(14, 9), + // (14,35): error CS8168: Cannot return local 's1' by reference because it is not a ref local + // GetReadonlyReference3(out s1).RefField = ref value; // 6 + Diagnostic(ErrorCode.ERR_RefReturnLocal, "s1").WithArguments("s1").WithLocation(14, 35)); } [WorkItem(66128, "https://github.com/dotnet/roslyn/issues/66128")]