From 915943aebc2617b61d0b1bd0f1145026641e47e5 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 1 Oct 2024 15:22:59 +0200 Subject: [PATCH 1/2] Allow accessing ref fields through rvalues --- .../Portable/Binder/Binder.ValueChecks.cs | 32 ++ .../Test/Semantic/Semantics/RefFieldTests.cs | 410 +++++++++++++++++- 2 files changed, 418 insertions(+), 24 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index d1cc8b15db4f5..6f91bd536b786 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -1398,6 +1398,38 @@ private bool CheckFieldValueKind(SyntaxNode node, BoundFieldAccess fieldAccess, } } + if (RequiresRefOrOut(valueKind)) + { + switch (fieldSymbol.RefKind) + { + case RefKind.None: + break; + case RefKind.Ref: + // ref/out access to a ref field is fine regardless of the receiver + return true; + case RefKind.RefReadOnly: + ReportReadOnlyError(fieldSymbol, node, valueKind, checkingReceiver, diagnostics); + return false; + default: + throw ExceptionUtilities.UnexpectedValue(fieldSymbol.RefKind); + } + } + + if (RequiresReferenceToLocation(valueKind)) + { + switch (fieldSymbol.RefKind) + { + case RefKind.None: + break; + case RefKind.Ref: + case RefKind.RefReadOnly: + // ref readonly access to a ref (readonly) field is fine regardless of the receiver + return true; + default: + throw ExceptionUtilities.UnexpectedValue(fieldSymbol.RefKind); + } + } + // r/w fields that are static or belong to reference types are writeable and returnable if (fieldSymbol.IsStatic || fieldSymbol.ContainingType.IsReferenceType) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs index 3b57835da5883..524e3e7ab5cb4 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs @@ -30156,7 +30156,7 @@ static void Test1() [WorkItem("https://github.com/dotnet/roslyn/issues/75082")] public void RefField_AsRefArgument_03() { - var comp = CreateCompilation(""" + var verifier = CompileAndVerify(""" ref struct S { public ref readonly int F1; @@ -30171,13 +30171,24 @@ static void Test1() } } """, + verify: Verification.Skipped, targetFramework: TargetFramework.NetCoreApp); - comp.VerifyDiagnostics( - // (11,14): error CS1612: Cannot modify the return value of 'S.GetS()' because it is not a variable - // M(in GetS().F1); - Diagnostic(ErrorCode.ERR_ReturnNotLValue, "GetS()").WithArguments("S.GetS()").WithLocation(11, 14) - ); + verifier.VerifyDiagnostics(); + + verifier.VerifyIL("S.Test1", """ + { + // Code size 19 (0x13) + .maxstack 1 + .locals init (S V_0) + IL_0000: call "S S.GetS()" + IL_0005: stloc.0 + IL_0006: ldloca.s V_0 + IL_0008: ldfld "ref readonly int S.F1" + IL_000d: call "void S.M(ref readonly int)" + IL_0012: ret + } + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75035")] @@ -30212,7 +30223,7 @@ static void Test1() [WorkItem("https://github.com/dotnet/roslyn/issues/75082")] public void RefField_AsRefArgument_05() { - var comp = CreateCompilation(""" + var verifier = CompileAndVerify(""" ref struct S { public ref readonly int F1; @@ -30227,13 +30238,24 @@ static void Test1() } } """, + verify: Verification.Skipped, targetFramework: TargetFramework.NetCoreApp); - comp.VerifyDiagnostics( - // (11,14): error CS1612: Cannot modify the return value of 'S.GetS()' because it is not a variable - // M(in GetS().F1); - Diagnostic(ErrorCode.ERR_ReturnNotLValue, "GetS()").WithArguments("S.GetS()").WithLocation(11, 14) - ); + verifier.VerifyDiagnostics(); + + verifier.VerifyIL("S.Test1", """ + { + // Code size 19 (0x13) + .maxstack 1 + .locals init (S V_0) + IL_0000: call "S S.GetS()" + IL_0005: stloc.0 + IL_0006: ldloca.s V_0 + IL_0008: ldfld "ref readonly int S.F1" + IL_000d: call "void S.M(in int)" + IL_0012: ret + } + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75035")] @@ -30360,7 +30382,7 @@ .locals init (S V_0) [WorkItem("https://github.com/dotnet/roslyn/issues/75082")] public void RefField_AsRefArgument_09() { - var comp = CreateCompilation(""" + var verifier = CompileAndVerify(""" ref struct S { public readonly ref int F1; @@ -30375,13 +30397,24 @@ static void Test1() } } """, + verify: Verification.Skipped, targetFramework: TargetFramework.NetCoreApp); - comp.VerifyDiagnostics( - // (11,14): error CS1612: Cannot modify the return value of 'S.GetS()' because it is not a variable - // M(in GetS().F1); - Diagnostic(ErrorCode.ERR_ReturnNotLValue, "GetS()").WithArguments("S.GetS()").WithLocation(11, 14) - ); + verifier.VerifyDiagnostics(); + + verifier.VerifyIL("S.Test1", """ + { + // Code size 19 (0x13) + .maxstack 1 + .locals init (S V_0) + IL_0000: call "S S.GetS()" + IL_0005: stloc.0 + IL_0006: ldloca.s V_0 + IL_0008: ldfld "ref int S.F1" + IL_000d: call "void S.M(ref readonly int)" + IL_0012: ret + } + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75035")] @@ -30432,7 +30465,7 @@ .locals init (S V_0) [WorkItem("https://github.com/dotnet/roslyn/issues/75082")] public void RefField_AsRefArgument_11() { - var comp = CreateCompilation(""" + var verifier = CompileAndVerify(""" ref struct S { public readonly ref int F1; @@ -30447,13 +30480,24 @@ static void Test1() } } """, + verify: Verification.Skipped, targetFramework: TargetFramework.NetCoreApp); - comp.VerifyDiagnostics( - // (11,14): error CS1612: Cannot modify the return value of 'S.GetS()' because it is not a variable - // M(in GetS().F1); - Diagnostic(ErrorCode.ERR_ReturnNotLValue, "GetS()").WithArguments("S.GetS()").WithLocation(11, 14) - ); + verifier.VerifyDiagnostics(); + + verifier.VerifyIL("S.Test1", """ + { + // Code size 19 (0x13) + .maxstack 1 + .locals init (S V_0) + IL_0000: call "S S.GetS()" + IL_0005: stloc.0 + IL_0006: ldloca.s V_0 + IL_0008: ldfld "ref int S.F1" + IL_000d: call "void S.M(in int)" + IL_0012: ret + } + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75035")] @@ -30496,5 +30540,323 @@ .locals init (S V_0) } "); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_RefOfRef() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(ref GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_InOfRef() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(in GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_InOfRef_Nested() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref int F; + + public R GetR() => default; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(in GetValue().GetR().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_OutOfRef() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(out int x) => throw null; + + static void Test() + { + M(out GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRef_ToRefReadonly() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (13,11): warning CS9192: Argument 1 should be passed with 'ref' or 'in' keyword + // M(GetValue().F); + Diagnostic(ErrorCode.WRN_ArgExpectedRefOrIn, "GetValue().F").WithArguments("1").WithLocation(13, 11)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRef_ToIn() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(in int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRef_ToRef() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref int F; + + public static R GetValue() => default; + + static void M(ref int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (13,11): error CS1620: Argument 1 must be passed with the 'ref' keyword + // M(GetValue().F); + Diagnostic(ErrorCode.ERR_BadArgRef, "GetValue().F").WithArguments("1", "ref").WithLocation(13, 11)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_RefOfRefReadonly() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(ref GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (11,15): error CS8329: Cannot use field 'F' as a ref or out value because it is a readonly variable + // M(ref GetValue().F); + Diagnostic(ErrorCode.ERR_RefReadonlyNotField, "GetValue().F").WithArguments("field", "F").WithLocation(11, 15)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_InOfRefReadonly() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(in GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_OutOfRefReadonly() + { + var comp = CreateCompilation(""" + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(out int x) => throw null; + + static void Test() + { + M(out GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (11,15): error CS8329: Cannot use field 'F' as a ref or out value because it is a readonly variable + // M(out GetValue().F); + Diagnostic(ErrorCode.ERR_RefReadonlyNotField, "GetValue().F").WithArguments("field", "F").WithLocation(11, 15)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRefReadonly_ToRefReadonly() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(ref readonly int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (13,11): warning CS9195: Argument 1 should be passed with the 'in' keyword + // M(GetValue().F); + Diagnostic(ErrorCode.WRN_ArgExpectedIn, "GetValue().F").WithArguments("1").WithLocation(13, 11)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRefReadonly_ToIn() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(in int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75082")] + public void ThroughValue_ValueOfRefReadonly_ToRef() + { + var comp = CreateCompilation(""" + #pragma warning disable CS0649 // Field 'R.F' is never assigned to, and will always have its default value 0 + + ref struct R + { + public ref readonly int F; + + public static R GetValue() => default; + + static void M(ref int x) { } + + static void Test() + { + M(GetValue().F); + } + } + """, + targetFramework: TargetFramework.NetCoreApp); + comp.VerifyDiagnostics( + // (13,11): error CS1620: Argument 1 must be passed with the 'ref' keyword + // M(GetValue().F); + Diagnostic(ErrorCode.ERR_BadArgRef, "GetValue().F").WithArguments("1", "ref").WithLocation(13, 11)); + } } } From 6628b0b770db07757c2c2c4178cbc46b0d7f099a Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 2 Oct 2024 10:53:27 +0200 Subject: [PATCH 2/2] Remove unnecessary block --- .../Portable/Binder/Binder.ValueChecks.cs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 6f91bd536b786..aea6580634b3a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -1398,23 +1398,6 @@ private bool CheckFieldValueKind(SyntaxNode node, BoundFieldAccess fieldAccess, } } - if (RequiresRefOrOut(valueKind)) - { - switch (fieldSymbol.RefKind) - { - case RefKind.None: - break; - case RefKind.Ref: - // ref/out access to a ref field is fine regardless of the receiver - return true; - case RefKind.RefReadOnly: - ReportReadOnlyError(fieldSymbol, node, valueKind, checkingReceiver, diagnostics); - return false; - default: - throw ExceptionUtilities.UnexpectedValue(fieldSymbol.RefKind); - } - } - if (RequiresReferenceToLocation(valueKind)) { switch (fieldSymbol.RefKind)