From 86c4d1355ee759b3452f971d348c2c0cc665b4dd Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 2 Jan 2023 01:52:18 -0800 Subject: [PATCH 1/4] Emit defensive copy for constrained call on `in` parameter --- .../CSharp/Portable/CodeGen/EmitExpression.cs | 4 +- .../Emit/CodeGen/CodeGenInParametersTests.cs | 188 ++++++++++++++++++ .../DefaultInterfaceImplementationTests.cs | 79 ++++++++ 3 files changed, 269 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index 6ff3fda3360ed..d88b074caee3b 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -1626,7 +1626,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) } else { - // calling a method defined in a base class. + // calling a method defined in a base class or interface. // When calling a method that is virtual in metadata on a struct receiver, // we use a constrained virtual call. If possible, it will skip boxing. @@ -1634,7 +1634,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) { // NB: all methods that a struct could inherit from bases are non-mutating // treat receiver as ReadOnly - tempOpt = EmitReceiverRef(receiver, AddressKind.ReadOnly); + tempOpt = EmitReceiverRef(receiver, methodContainingType.IsInterface ? AddressKind.Writeable : AddressKind.ReadOnly); callKind = CallKind.ConstrainedCallVirt; } else diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs index a43535d8ea44d..d808278bce0c1 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs @@ -4386,5 +4386,193 @@ .locals init (Test V_0) } "); } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void ConstrainedCallOnInParameter() + { + var source = @" +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class C +{ + public static void Main() + { + S value = new(); + ref readonly S valueRef = ref value; + Console.Write(valueRef); + M(in valueRef); + Console.Write(valueRef); + } + public static void M(in S value) + { + foreach (var x in value) { } + } +} + +public struct S : IEnumerable +{ + int a; + public readonly override string ToString() => a.ToString(); + private IEnumerator GetEnumerator() => Enumerable.Range(0, ++a).GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +}"; + var verifier = CompileAndVerify(source, expectedOutput: "00"); + // Note: we use a temp instead of directly doing a constrained call on `in` parameter + verifier.VerifyIL("C.M", """ +{ + // Code size 51 (0x33) + .maxstack 1 + .locals init (System.Collections.Generic.IEnumerator V_0, + S V_1) + IL_0000: ldarg.0 + IL_0001: ldobj "S" + IL_0006: stloc.1 + IL_0007: ldloca.s V_1 + IL_0009: constrained. "S" + IL_000f: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0014: stloc.0 + .try + { + IL_0015: br.s IL_001e + IL_0017: ldloc.0 + IL_0018: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_001d: pop + IL_001e: ldloc.0 + IL_001f: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0024: brtrue.s IL_0017 + IL_0026: leave.s IL_0032 + } + finally + { + IL_0028: ldloc.0 + IL_0029: brfalse.s IL_0031 + IL_002b: ldloc.0 + IL_002c: callvirt "void System.IDisposable.Dispose()" + IL_0031: endfinally + } + IL_0032: ret +} +"""); + } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void ConstrainedCallOnInParameter_ConstrainedGenericReceiver() + { + var source = @" +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class C +{ + public static void Main() + { + S value = new(); + ref readonly S valueRef = ref value; + Console.Write(valueRef); + M(in valueRef); + Console.Write(valueRef); + } + public static void M(in T value) where T : struct, IEnumerable + { + foreach (var x in value) { } + } +} + +public struct S : IEnumerable +{ + int a; + public readonly override string ToString() => a.ToString(); + private IEnumerator GetEnumerator() => Enumerable.Range(0, ++a).GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +}"; + var verifier = CompileAndVerify(source, expectedOutput: "00"); + verifier.VerifyIL("C.M(in T)", """ +{ + // Code size 51 (0x33) + .maxstack 1 + .locals init (System.Collections.Generic.IEnumerator V_0, + T V_1) + IL_0000: ldarg.0 + IL_0001: ldobj "T" + IL_0006: stloc.1 + IL_0007: ldloca.s V_1 + IL_0009: constrained. "T" + IL_000f: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0014: stloc.0 + .try + { + IL_0015: br.s IL_001e + IL_0017: ldloc.0 + IL_0018: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_001d: pop + IL_001e: ldloc.0 + IL_001f: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0024: brtrue.s IL_0017 + IL_0026: leave.s IL_0032 + } + finally + { + IL_0028: ldloc.0 + IL_0029: brfalse.s IL_0031 + IL_002b: ldloc.0 + IL_002c: callvirt "void System.IDisposable.Dispose()" + IL_0031: endfinally + } + IL_0032: ret +} +"""); + } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void InvokeStructToStringOverrideOnInParameter() + { + var text = @" +using System; + +class C +{ + public static void Main() + { + S1 s = new S1(); + Console.Write(M(in s)); + Console.Write(M(in s)); + } + static string M(in S1 s) + { + return s.ToString(); + } +} +struct S1 +{ + int i; + public override string ToString() => (i++).ToString(); +} +"; + + var comp = CompileAndVerify(text, expectedOutput: "00"); + + comp.VerifyIL("C.M", """ +{ + // Code size 21 (0x15) + .maxstack 1 + .locals init (S1 V_0) + IL_0000: ldarg.0 + IL_0001: ldobj "S1" + IL_0006: stloc.0 + IL_0007: ldloca.s V_0 + IL_0009: constrained. "S1" + IL_000f: callvirt "string object.ToString()" + IL_0014: ret +} +"""); + } + } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/DefaultInterfaceImplementationTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/DefaultInterfaceImplementationTests.cs index a75973328e7ec..279ae78619530 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/DefaultInterfaceImplementationTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/DefaultInterfaceImplementationTests.cs @@ -68913,5 +68913,84 @@ event System.Action E2 {add{} remove{}} Diagnostic(ErrorCode.ERR_ComImportWithImpl, "int").WithArguments("I1.implicit operator int(I1)", "I1").WithLocation(28, 30) ); } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void ConstrainedCallOnInParameter_DefaultImplementation() + { + var source = @" +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class C +{ + public static void Main() + { + S value = new(); + ref readonly S valueRef = ref value; + Console.Write(valueRef); + M(in valueRef); + Console.Write(valueRef); + } + public static void M(in S value) + { + foreach (var x in value) { } + } +} + +public interface MyEnumerable : IEnumerable +{ + IEnumerator GetEnumeratorCore(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumeratorCore(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumeratorCore(); +} + +public struct S : MyEnumerable +{ + int a; + public readonly override string ToString() => a.ToString(); + public IEnumerator GetEnumeratorCore() => Enumerable.Range(0, ++a).GetEnumerator(); +}"; + var verifier = CompileAndVerify(source, targetFramework: TargetFramework.Net70, + expectedOutput: Execute(isStatic: false) ? "00" : null, + verify: Verify(isStatic: false)); + + verifier.VerifyIL("C.M", """ +{ + // Code size 51 (0x33) + .maxstack 1 + .locals init (System.Collections.Generic.IEnumerator V_0, + S V_1) + IL_0000: ldarg.0 + IL_0001: ldobj "S" + IL_0006: stloc.1 + IL_0007: ldloca.s V_1 + IL_0009: constrained. "S" + IL_000f: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0014: stloc.0 + .try + { + IL_0015: br.s IL_001e + IL_0017: ldloc.0 + IL_0018: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_001d: pop + IL_001e: ldloc.0 + IL_001f: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0024: brtrue.s IL_0017 + IL_0026: leave.s IL_0032 + } + finally + { + IL_0028: ldloc.0 + IL_0029: brfalse.s IL_0031 + IL_002b: ldloc.0 + IL_002c: callvirt "void System.IDisposable.Dispose()" + IL_0031: endfinally + } + IL_0032: ret +} +"""); + } } } From c711d2b86501da84bf30f2ecc276f0210c294370 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 5 Jan 2023 14:45:40 -0800 Subject: [PATCH 2/4] Address feedback --- .../CSharp/Portable/CodeGen/EmitExpression.cs | 4 +- .../Emit/CodeGen/CodeGenInParametersTests.cs | 224 ++++++++ .../CodeGen/CodeGenReadonlyStructTests.cs | 54 +- .../Emit/CodeGen/CodeGenStructsAndEnum.cs | 18 +- .../Semantic/Semantics/RecordStructTests.cs | 19 +- .../Test/Emit/CodeGen/CodeGenCallTests.vb | 502 +++++++++++++++++- .../Source/MeMyBaseMyClassTests.vb | 75 +++ 7 files changed, 849 insertions(+), 47 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index d88b074caee3b..1605b8c0a0282 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -1632,9 +1632,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) // we use a constrained virtual call. If possible, it will skip boxing. if (method.IsMetadataVirtual()) { - // NB: all methods that a struct could inherit from bases are non-mutating - // treat receiver as ReadOnly - tempOpt = EmitReceiverRef(receiver, methodContainingType.IsInterface ? AddressKind.Writeable : AddressKind.ReadOnly); + tempOpt = EmitReceiverRef(receiver, AddressKind.Writeable); callKind = CallKind.ConstrainedCallVirt; } else diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs index d808278bce0c1..a6e5015e46677 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs @@ -4530,6 +4530,171 @@ .locals init (System.Collections.Generic.IEnumerator V_0, """); } + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void ConstrainedCallOnReadonlyField() + { + var source = @" +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class C +{ + public static void Main() + { + S s = new(); + var d = new D(s); + d.M(); + d.M(); + } +} + +public class D +{ + readonly S field; + public D(S s) { field = s; } + + public void M() + { + foreach (var x in field) { } + System.Console.Write(field.ToString()); + } +} + +public struct S : IEnumerable +{ + int a; + public readonly override string ToString() => a.ToString(); + private IEnumerator GetEnumerator() => Enumerable.Range(0, ++a).GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +}"; + var verifier = CompileAndVerify(source, expectedOutput: "00", verify: Verification.FailsPEVerify); + // Note: we use a temp instead of directly doing a constrained call on readonly field + verifier.VerifyIL("D.M", """ +{ + // Code size 73 (0x49) + .maxstack 1 + .locals init (System.Collections.Generic.IEnumerator V_0, + S V_1) + IL_0000: ldarg.0 + IL_0001: ldfld "S D.field" + IL_0006: stloc.1 + IL_0007: ldloca.s V_1 + IL_0009: constrained. "S" + IL_000f: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0014: stloc.0 + .try + { + IL_0015: br.s IL_001e + IL_0017: ldloc.0 + IL_0018: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_001d: pop + IL_001e: ldloc.0 + IL_001f: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0024: brtrue.s IL_0017 + IL_0026: leave.s IL_0032 + } + finally + { + IL_0028: ldloc.0 + IL_0029: brfalse.s IL_0031 + IL_002b: ldloc.0 + IL_002c: callvirt "void System.IDisposable.Dispose()" + IL_0031: endfinally + } + IL_0032: ldarg.0 + IL_0033: ldflda "S D.field" + IL_0038: constrained. "S" + IL_003e: callvirt "string object.ToString()" + IL_0043: call "void System.Console.Write(string)" + IL_0048: ret +} +"""); + } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void ConstrainedCallOnField() + { + var source = @" +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class C +{ + public static void Main() + { + S s = new(); + var d = new D(s); + d.M(); + d.M(); + } +} + +public class D +{ + S field; + public D(S s) { field = s; } + + public void M() + { + foreach (var x in field) { } + System.Console.Write(field.ToString()); + } +} + +public struct S : IEnumerable +{ + int a; + public readonly override string ToString() => a.ToString(); + private IEnumerator GetEnumerator() => Enumerable.Range(0, ++a).GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); +}"; + var verifier = CompileAndVerify(source, expectedOutput: "12"); + // Note: we do a constrained call directly on the field + verifier.VerifyIL("D.M", """ +{ + // Code size 70 (0x46) + .maxstack 1 + .locals init (System.Collections.Generic.IEnumerator V_0) + IL_0000: ldarg.0 + IL_0001: ldflda "S D.field" + IL_0006: constrained. "S" + IL_000c: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0011: stloc.0 + .try + { + IL_0012: br.s IL_001b + IL_0014: ldloc.0 + IL_0015: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_001a: pop + IL_001b: ldloc.0 + IL_001c: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0021: brtrue.s IL_0014 + IL_0023: leave.s IL_002f + } + finally + { + IL_0025: ldloc.0 + IL_0026: brfalse.s IL_002e + IL_0028: ldloc.0 + IL_0029: callvirt "void System.IDisposable.Dispose()" + IL_002e: endfinally + } + IL_002f: ldarg.0 + IL_0030: ldflda "S D.field" + IL_0035: constrained. "S" + IL_003b: callvirt "string object.ToString()" + IL_0040: call "void System.Console.Write(string)" + IL_0045: ret +} +"""); + } + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] public void InvokeStructToStringOverrideOnInParameter() { @@ -4574,5 +4739,64 @@ .locals init (S1 V_0) """); } + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void InvokeAddedStructToStringOverrideOnInParameter() + { + var libOrig_cs = """ +public struct S +{ + int i; + public void Report() { throw null; } +} +"""; + var libOrig = CreateCompilation(libOrig_cs, assemblyName: "lib"); + + var libChanged_cs = """ +public struct S +{ + int i; + public override string ToString() => (i++).ToString(); + public void Report() { System.Console.Write("RAN "); } +} +"""; + var libChanged = CreateCompilation(libChanged_cs, assemblyName: "lib"); + + var libUser_cs = """ +public class C +{ + public static string M(in S s) + { + return s.ToString(); + } +} +"""; + var libUser = CreateCompilation(libUser_cs, references: new[] { libOrig.EmitToImageReference() }); + CompileAndVerify(libUser).VerifyIL("C.M", """ +{ + // Code size 21 (0x15) + .maxstack 1 + .locals init (S V_0) + IL_0000: ldarg.0 + IL_0001: ldobj "S" + IL_0006: stloc.0 + IL_0007: ldloca.s V_0 + IL_0009: constrained. "S" + IL_000f: callvirt "string object.ToString()" + IL_0014: ret +} +"""); + + var src = """ +using System; + +S s = new S(); +s.Report(); +Console.Write(C.M(in s)); +Console.Write(C.M(in s)); +"""; + + var comp = CreateCompilation(src, references: new[] { libChanged.EmitToImageReference(), libUser.EmitToImageReference() }); + CompileAndVerify(comp, expectedOutput: "RAN 00"); + } } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs index 179cdbc13fb3a..adcc4abab1aa2 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs @@ -514,18 +514,22 @@ public void Test() comp.VerifyIL("Program.S1.Test()", @" { - // Code size 39 (0x27) + // Code size 47 (0x2f) .maxstack 1 + .locals init (Program.S1 V_0) IL_0000: ldarg.0 IL_0001: ldobj ""Program.S1"" IL_0006: box ""Program.S1"" IL_000b: call ""System.Type object.GetType()"" IL_0010: call ""void System.Console.Write(object)"" IL_0015: ldarg.0 - IL_0016: constrained. ""Program.S1"" - IL_001c: callvirt ""string object.ToString()"" - IL_0021: call ""void System.Console.Write(string)"" - IL_0026: ret + IL_0016: ldobj ""Program.S1"" + IL_001b: stloc.0 + IL_001c: ldloca.s V_0 + IL_001e: constrained. ""Program.S1"" + IL_0024: callvirt ""string object.ToString()"" + IL_0029: call ""void System.Console.Write(string)"" + IL_002e: ret }"); } @@ -2162,30 +2166,40 @@ readonly void M() var comp = CompileAndVerify(csharp); comp.VerifyDiagnostics(); - // ToString/GetHashCode/Equals should pass the address of 'this' (not a temp). GetType should box 'this'. + // GetType should box 'this'. comp.VerifyIL("S.M", @" { - // Code size 58 (0x3a) + // Code size 82 (0x52) .maxstack 2 + .locals init (S V_0) IL_0000: ldarg.0 IL_0001: ldobj ""S"" IL_0006: box ""S"" IL_000b: call ""System.Type object.GetType()"" IL_0010: pop IL_0011: ldarg.0 - IL_0012: constrained. ""S"" - IL_0018: callvirt ""string object.ToString()"" - IL_001d: pop - IL_001e: ldarg.0 - IL_001f: constrained. ""S"" - IL_0025: callvirt ""int object.GetHashCode()"" - IL_002a: pop - IL_002b: ldarg.0 - IL_002c: ldnull - IL_002d: constrained. ""S"" - IL_0033: callvirt ""bool object.Equals(object)"" - IL_0038: pop - IL_0039: ret + IL_0012: ldobj ""S"" + IL_0017: stloc.0 + IL_0018: ldloca.s V_0 + IL_001a: constrained. ""S"" + IL_0020: callvirt ""string object.ToString()"" + IL_0025: pop + IL_0026: ldarg.0 + IL_0027: ldobj ""S"" + IL_002c: stloc.0 + IL_002d: ldloca.s V_0 + IL_002f: constrained. ""S"" + IL_0035: callvirt ""int object.GetHashCode()"" + IL_003a: pop + IL_003b: ldarg.0 + IL_003c: ldobj ""S"" + IL_0041: stloc.0 + IL_0042: ldloca.s V_0 + IL_0044: ldnull + IL_0045: constrained. ""S"" + IL_004b: callvirt ""bool object.Equals(object)"" + IL_0050: pop + IL_0051: ret }"); } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenStructsAndEnum.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenStructsAndEnum.cs index a5f73b415a144..6a1f4d83a1c4b 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenStructsAndEnum.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenStructsAndEnum.cs @@ -1554,17 +1554,19 @@ struct S1 var compilation = CompileAndVerify(source, expectedOutput: "S1", verify: Verification.Skipped); - compilation.VerifyIL("Program.Main", -@" + compilation.VerifyIL("Program.Main", @" { - // Code size 27 (0x1b) + // Code size 30 (0x1e) .maxstack 1 + .locals init (S1 V_0) IL_0000: newobj ""C1..ctor()"" - IL_0005: ldflda ""S1 C1.field"" - IL_000a: constrained. ""S1"" - IL_0010: callvirt ""string object.ToString()"" - IL_0015: call ""void System.Console.WriteLine(string)"" - IL_001a: ret + IL_0005: ldfld ""S1 C1.field"" + IL_000a: stloc.0 + IL_000b: ldloca.s V_0 + IL_000d: constrained. ""S1"" + IL_0013: callvirt ""string object.ToString()"" + IL_0018: call ""void System.Console.WriteLine(string)"" + IL_001d: ret } "); compilation = CompileAndVerify(source, expectedOutput: "S1", parseOptions: TestOptions.Regular.WithPEVerifyCompatFeature()); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordStructTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordStructTests.cs index 612de6a91af98..5fea8310aba7c 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordStructTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordStructTests.cs @@ -5372,21 +5372,24 @@ record struct C1 v.VerifyIL("C1." + WellKnownMemberNames.PrintMembersMethodName, @" { - // Code size 38 (0x26) + // Code size 41 (0x29) .maxstack 2 + .locals init (int V_0) IL_0000: ldarg.1 IL_0001: ldstr ""field = "" IL_0006: callvirt ""System.Text.StringBuilder System.Text.StringBuilder.Append(string)"" IL_000b: pop IL_000c: ldarg.1 IL_000d: ldarg.0 - IL_000e: ldflda ""int C1.field"" - IL_0013: constrained. ""int"" - IL_0019: callvirt ""string object.ToString()"" - IL_001e: callvirt ""System.Text.StringBuilder System.Text.StringBuilder.Append(string)"" - IL_0023: pop - IL_0024: ldc.i4.1 - IL_0025: ret + IL_000e: ldfld ""int C1.field"" + IL_0013: stloc.0 + IL_0014: ldloca.s V_0 + IL_0016: constrained. ""int"" + IL_001c: callvirt ""string object.ToString()"" + IL_0021: callvirt ""System.Text.StringBuilder System.Text.StringBuilder.Append(string)"" + IL_0026: pop + IL_0027: ldc.i4.1 + IL_0028: ret } "); } diff --git a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenCallTests.vb b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenCallTests.vb index f4ed7c2049db4..c16be81b87975 100644 --- a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenCallTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenCallTests.vb @@ -2,16 +2,8 @@ ' The .NET Foundation licenses this file to you under the MIT license. ' See the LICENSE file in the project root for more information. -Imports System.Collections.Immutable -Imports System.Reflection -Imports System.Xml.Linq -Imports Microsoft.CodeAnalysis -Imports Microsoft.CodeAnalysis.Test.Resources.Proprietary Imports Microsoft.CodeAnalysis.Test.Utilities -Imports Microsoft.CodeAnalysis.VisualBasic -Imports Microsoft.CodeAnalysis.VisualBasic.Symbols Imports Roslyn.Test.Utilities -Imports Roslyn.Test.Utilities.TestMetadata Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests @@ -7735,5 +7727,499 @@ Position set for item '-4' ]]>) End Sub + + + Public Sub InvokeAddedStructToStringOverrideOnReadOnlyField() + Dim libOrig_vb = + + +Public Structure S + Public i As Integer + + Public Sub Report() + Throw New System.Exception() + End Sub +End Structure + + + Dim libOrig = CreateCompilation(libOrig_vb, assemblyName:="lib") + libOrig.AssertTheseDiagnostics() + + Dim libChanged_vb = + + +Public Structure S + Public i As Integer + + Public Overrides Function ToString() As String + Dim result = i.ToString() + i = i + 1 + Return result + End Function + + Public Sub Report() + System.Console.Write("RAN ") + End Sub +End Structure + + + Dim libChanged = CreateCompilation(libChanged_vb, assemblyName:="lib") + libChanged.AssertTheseDiagnostics() + + Dim libUser_vb = + + +Public Class C + Public ReadOnly field As S + + Public Sub New(s As S) + field = s + End Sub + + Public Sub M() + System.Console.Write(field.ToString()) + System.Console.Write(field.ToString()) + End Sub +End Class + + + Dim libUser = CreateCompilation(libUser_vb, references:={libOrig.EmitToImageReference()}) + libUser.AssertTheseDiagnostics() + + CompileAndVerify(libUser).VerifyIL("C.M", ) + + Dim src = + + +Class D + Public Shared Sub Main() + Dim s = New S() + s.Report() + Dim c = New C(s) + c.M() + End Sub +End Class + + + Dim comp = CreateCompilation(src, references:={libChanged.EmitToImageReference(), libUser.EmitToImageReference()}, options:=TestOptions.DebugExe) + comp.AssertTheseDiagnostics() + CompileAndVerify(comp, expectedOutput:="RAN 00") + End Sub + + + + Public Sub InvokeAddedStructToStringOverrideOnField() + Dim libOrig_vb = + + +Public Structure S + Public i As Integer + + Public Sub Report() + Throw New System.Exception() + End Sub +End Structure + + + Dim libOrig = CreateCompilation(libOrig_vb, assemblyName:="lib") + libOrig.AssertTheseDiagnostics() + + Dim libChanged_vb = + + +Public Structure S + Public i As Integer + + Public Overrides Function ToString() As String + Dim result = i.ToString() + i = i + 1 + Return result + End Function + + Public Sub Report() + System.Console.Write("RAN ") + End Sub +End Structure + + + Dim libChanged = CreateCompilation(libChanged_vb, assemblyName:="lib") + libChanged.AssertTheseDiagnostics() + + Dim libUser_vb = + + +Public Class C + Public field As S + + Public Sub New(s As S) + field = s + End Sub + + Public Sub M() + System.Console.Write(field.ToString()) + System.Console.Write(field.ToString()) + End Sub +End Class + + + Dim libUser = CreateCompilation(libUser_vb, references:={libOrig.EmitToImageReference()}) + libUser.AssertTheseDiagnostics() + + CompileAndVerify(libUser).VerifyIL("C.M", ) + + Dim src = + + +Class D + Public Shared Sub Main() + Dim s = New S() + s.Report() + Dim c = New C(s) + c.M() + End Sub +End Class + + + Dim comp = CreateCompilation(src, references:={libChanged.EmitToImageReference(), libUser.EmitToImageReference()}, options:=TestOptions.DebugExe) + comp.AssertTheseDiagnostics() + CompileAndVerify(comp, expectedOutput:="RAN 01") + End Sub + + + + Public Sub InvokeStructToAddedStringOverrideOnRefParameter() + Dim libOrig_vb = + + +Public Structure S + Public i As Integer + + Public Sub Report() + Throw New System.Exception() + End Sub +End Structure + + + Dim libOrig = CreateCompilation(libOrig_vb, assemblyName:="lib") + libOrig.AssertTheseDiagnostics() + + Dim libChanged_vb = + + +Public Structure S + Public i As Integer + + Public Overrides Function ToString() As String + Dim result = i.ToString() + i = i + 1 + Return result + End Function + + Public Sub Report() + System.Console.Write("RAN ") + End Sub +End Structure + + + Dim libChanged = CreateCompilation(libChanged_vb, assemblyName:="lib") + libChanged.AssertTheseDiagnostics() + + Dim libUser_vb = + + +Public Class C + Public Sub M(ByRef s As S) + System.Console.Write(s.ToString()) + System.Console.Write(s.ToString()) + End Sub +End Class + + + Dim libUser = CreateCompilation(libUser_vb, references:={libOrig.EmitToImageReference()}) + libUser.AssertTheseDiagnostics() + + CompileAndVerify(libUser).VerifyIL("C.M", ) + + Dim src = + + +Class D + Public Shared Sub Main() + Dim s = New S() + s.Report() + Dim c = New C() + c.M(s) + End Sub +End Class + + + Dim comp = CreateCompilation(src, references:={libChanged.EmitToImageReference(), libUser.EmitToImageReference()}, options:=TestOptions.DebugExe) + comp.AssertTheseDiagnostics() + CompileAndVerify(comp, expectedOutput:="RAN 01") + End Sub + + + + Public Sub ForEachOnReadOnlyField() + Dim src = + + +Imports System.Collections +Imports System.Collections.Generic + +Class C + Public Shared Sub Main() + Dim d = New D() + d.M() + d.M() + End Sub +End Class + +Class D + ReadOnly field As S + + Public Sub M() + For Each x In field + Next + + System.Console.Write(field.ToString()) + End Sub +End Class + +Structure S + Implements IEnumerable(Of Integer) + + Public a As Integer + + Public Overrides Function ToString() As String + Return a.ToString() + End Function + + Private Iterator Function GetEnumerator() As IEnumerator(Of Integer) _ + Implements IEnumerable(Of Integer).GetEnumerator + + a = a + 1 + Yield 1 + End Function + + Private Function GetEnumerator2() As IEnumerator _ + Implements IEnumerable.GetEnumerator + + Return GetEnumerator() + End Function +End Structure + + + Dim comp = CreateCompilation(src, options:=TestOptions.DebugExe) + comp.AssertTheseDiagnostics() + CompileAndVerify(comp, expectedOutput:="00") + + CompileAndVerify(comp).VerifyIL("D.M", ) + End Sub + + + + Public Sub ForEachOnField() + Dim src = + + +Imports System.Collections +Imports System.Collections.Generic + +Class C + Public Shared Sub Main() + Dim d = New D() + d.M() + d.M() + End Sub +End Class + +Class D + Dim field As S + + Public Sub M() + For Each x In field + Next + + System.Console.Write(field.ToString()) + End Sub +End Class + +Structure S + Implements IEnumerable(Of Integer) + + Public a As Integer + + Public Overrides Function ToString() As String + Return a.ToString() + End Function + + Private Iterator Function GetEnumerator() As IEnumerator(Of Integer) _ + Implements IEnumerable(Of Integer).GetEnumerator + + a = a + 1 + Yield 1 + End Function + + Private Function GetEnumerator2() As IEnumerator _ + Implements IEnumerable.GetEnumerator + + Return GetEnumerator() + End Function +End Structure + + + Dim comp = CreateCompilation(src, options:=TestOptions.DebugExe) + comp.AssertTheseDiagnostics() + CompileAndVerify(comp, expectedOutput:="00") + + CompileAndVerify(comp).VerifyIL("D.M", ) + + End Sub + End Class End Namespace diff --git a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Source/MeMyBaseMyClassTests.vb b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Source/MeMyBaseMyClassTests.vb index af9fc219b43e2..e7c3e35b3c9c6 100644 --- a/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Source/MeMyBaseMyClassTests.vb +++ b/src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Source/MeMyBaseMyClassTests.vb @@ -1476,6 +1476,81 @@ End Structure ]]>) End Sub + + Public Sub MyClassUsedInStructureWithMutation() + CompileAndVerify( + + +Imports System +Structure S1 + Dim i As Integer + + Public Shared Sub Main() + Dim s = New S1() + s.Goo() + s.Goo() + End Sub + + Sub Goo() + Console.Write(MyClass.M()) + End Sub + + Function M() As String + i = i + 1 + Return i.ToString() + End Function +End Structure + +, expectedOutput:="12").VerifyIL("S1.Goo", ) + End Sub + + + Public Sub MyClassUsedInStructureWithMutationInOverride() + CompileAndVerify( + + +Imports System +Structure S1 + Dim i As Integer + + Public Shared Sub Main() + Dim s = New S1() + s.Goo() + s.Goo() + End Sub + + Sub Goo() + Console.Write(MyClass.ToString()) + End Sub + + Public Overrides Function ToString() As String + i = i + 1 + Return i.ToString() + End Function +End Structure + +, expectedOutput:="12").VerifyIL("S1.Goo", ) + End Sub + ' 'MyClass' is valid only within an instance method. Public Sub MyClassOnlyValidInInstanceMethod() From fa16a33520678ff0dbaab436ce5871e0d9b294ba Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 11 Jan 2023 14:09:03 -0800 Subject: [PATCH 3/4] Add test --- .../Emit/CodeGen/CodeGenInParametersTests.cs | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs index a6e5015e46677..66dd8a93f88fe 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs @@ -4798,5 +4798,85 @@ .locals init (S V_0) var comp = CreateCompilation(src, references: new[] { libChanged.EmitToImageReference(), libUser.EmitToImageReference() }); CompileAndVerify(comp, expectedOutput: "RAN 00"); } + + [Fact, WorkItem(66135, "https://github.com/dotnet/roslyn/issues/66135")] + public void InvokeAddedStructToStringOverrideOnReadonlyField() + { + var libOrig_cs = """ +public struct S +{ + int i; + public void Report() { throw null; } +} +"""; + var libOrig = CreateCompilation(libOrig_cs, assemblyName: "lib"); + + var libChanged_cs = """ +public struct S +{ + int i; + public override string ToString() => (i++).ToString(); + public void Report() { System.Console.Write($"Report{i} "); } +} +"""; + var libChanged = CreateCompilation(libChanged_cs, assemblyName: "lib"); + + var libUser_cs = """ +public class C +{ + readonly S field; + public C(int i) + { + field.ToString(); + } + public string M() + { + return field.ToString(); + } + public void Report() { field.Report(); } +} +"""; + var libUser = CreateCompilation(libUser_cs, references: new[] { libOrig.EmitToImageReference() }); + var verifier = CompileAndVerify(libUser); + verifier.VerifyIL("C.M", """ +{ + // Code size 21 (0x15) + .maxstack 1 + .locals init (S V_0) + IL_0000: ldarg.0 + IL_0001: ldfld "S C.field" + IL_0006: stloc.0 + IL_0007: ldloca.s V_0 + IL_0009: constrained. "S" + IL_000f: callvirt "string object.ToString()" + IL_0014: ret +} +"""); + + verifier.VerifyIL("C..ctor(int)", """ +{ + // Code size 25 (0x19) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: call "object..ctor()" + IL_0006: ldarg.0 + IL_0007: ldflda "S C.field" + IL_000c: constrained. "S" + IL_0012: callvirt "string object.ToString()" + IL_0017: pop + IL_0018: ret +} +"""); + + var src = """ +C c = new C(42); +c.Report(); +System.Console.Write(c.M()); +System.Console.Write(c.M()); +"""; + + var comp = CreateCompilation(src, references: new[] { libChanged.EmitToImageReference(), libUser.EmitToImageReference() }); + CompileAndVerify(comp, expectedOutput: "Report1 11"); + } } } From 73767e83c079b6ca5e986ff14c825ff2ac2ee7b4 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 11 Jan 2023 14:49:55 -0800 Subject: [PATCH 4/4] Add tracked issue --- .../CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs index adcc4abab1aa2..a1dc3eaf6f640 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs @@ -489,7 +489,7 @@ .locals init (Program.S1 V_0) } - [Fact] + [Fact, WorkItem(66365, "https://github.com/dotnet/roslyn/issues/66365")] public void InvokeOnThisBaseMethods() { var text = @" @@ -512,6 +512,8 @@ public void Test() var comp = CompileAndVerify(text, parseOptions: TestOptions.Regular, verify: Verification.Passes, expectedOutput: @"Program+S1Program+S1"); + // We may be able to optimize this case (ie. avoid defensive copy) since the struct and the caller are in the same module + // Tracked by https://github.com/dotnet/roslyn/issues/66365 comp.VerifyIL("Program.S1.Test()", @" { // Code size 47 (0x2f)