Skip to content

Avoid unnecessary defensive copy for constrained calls on readonly struct receivers #82432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stevenelliottjr wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −13
Open

Avoid unnecessary defensive copy for constrained calls on readonly struct receivers #82432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1903,11 +1903,15 @@ CallKind determineEmitReceiverStrategy(BoundCall call, out AddressKind? addressK
{
// calling a method defined in a base class or interface.

// When calling a method that is virtual in metadata on a struct receiver,
// 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.
if (method.IsMetadataVirtual())
{
addressKind = AddressKind.Writeable;
// For readonly value type receivers, we only need readonly access since
// readonly structs guarantee non-mutation for all their methods, and the
// constrained call either resolves to a non-mutating method or boxes the
// value (which copies it). Either way, the original receiver is not mutated.
addressKind = receiverType.IsReadOnly ? AddressKind.ReadOnly : AddressKind.Writeable;
callKind = CallKind.ConstrainedCallVirt;
}
else
Expand Down
49 changes: 38 additions & 11 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyStructTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,29 +512,56 @@ 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
Copy link
Member

@jjonescz jjonescz Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider marking the issue as closed by the PR (writing literally closes #66365 in the PR's description)

comp.VerifyIL("Program.S1.Test()", @"
{
// Code size 47 (0x2f)
// Code size 39 (0x27)
.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: 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
IL_0016: constrained. ""Program.S1""
IL_001c: callvirt ""string object.ToString()""
IL_0021: call ""void System.Console.Write(string)""
IL_0026: ret
}");
}

[Fact, WorkItem(76288, "https://github.com/dotnet/roslyn/issues/76288")]
public void NoDefensiveCopy_ReadonlyStructField_ConstrainedCall()
{
var text = @"
struct X
{
private Y a;
public readonly string W() => a.ToString();
}

readonly struct Y
{
}
";
var comp = CreateCompilation(text, options: TestOptions.ReleaseDll);
comp.VerifyDiagnostics(
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VerifyDiagnostics

We generally prefer combining VerifyDiagnostics with CompileAndVerify since it collects diagnostics anyway and collects all diagnostics.

// (4,15): warning CS0649: Field 'X.a' is never assigned to, and will always have its default value
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// (4,15): warning CS0649: Field 'X.a' is never assigned to, and will always have its default value

Consider suppressing the warning with #pragma

// private Y a;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "a").WithArguments("X.a", "").WithLocation(4, 15));
var compVerifier = CompileAndVerify(comp, verify: Verification.Passes);
compVerifier.VerifyIL("X.W()", @"
{
// Code size 18 (0x12)
.maxstack 1
IL_0000: ldarg.0
IL_0001: ldflda ""Y X.a""
IL_0006: constrained. ""Y""
IL_000c: callvirt ""string object.ToString()""
IL_0011: ret
}
");
}

[Fact]
public void AssignThis()
{
Expand Down
Loading