Skip to content
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

Jump to bottom

Always use a by-value temp to capture a reference type receiver #73631

Merged
merged 7 commits into from
Jun 3, 2024
Merged

Always use a by-value temp to capture a reference type receiver #73631

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ac052e
Add baseline test for rewritting receiver which is a ref to a referen…
jcouv May 22, 2024
99e5c2f
Always use a by-value temp to capture a reference type receiver
jcouv May 22, 2024
7f60e36
Add new test baselines
jcouv May 23, 2024
6656166
Update tests with fix
jcouv May 23, 2024
3262eda
Add WorkItem link
jcouv May 30, 2024
48af3ac
Remove redundant check
jcouv May 30, 2024
02d592d
Fix linked issue
jcouv May 30, 2024
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
27 changes: 17 additions & 10 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,17 +677,24 @@ private ImmutableArray<BoundExpression> VisitArgumentsAndCaptureReceiverIfNeeded
}
else
{
refKind = rewrittenReceiver.GetRefKind();

if (refKind == RefKind.None &&
!rewrittenReceiver.Type.IsReferenceType &&
Binder.HasHome(rewrittenReceiver,
Binder.AddressKind.Constrained,
_factory.CurrentFunction,
peVerifyCompatEnabled: false,
stackLocalsOpt: null))
if (rewrittenReceiver.Type.IsReferenceType)
Copy link
Member Author

@jcouv jcouv May 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Copying comment from extensions/receiver PR in case it is useful:

            // If we need to capture a receiver (for compound assignment, null-coalescing assignment, extension receiver adjustment, interpolation `this` capture, ...)
            // we need to make sure the capturing temp preserves the semantics of the receiver.
            //
            // For instance:
            //   Struct s = ...;
            //   s.Method(s.field = 42)
            // must be rewritten as:
            //   ref Struct temp = ref s;
            //   temp.Method(s.field = 42)
            // so that Method can have and observe the proper side-effects on the struct.
            //
            // Similarly:
            //   Class c = ...;
            //   ref Class c2 = ref c;
            //   Class otherC = null;
            //   c2.Method(c2 = ref otherC)
            // must be rewritten as:
            //   Class temp = c2; // dereferences to c
            //   temp.Method(c2 = ref otherC)
``` #Resolved

{
refKind = RefKind.Ref;
refKind = RefKind.None;
}
else
{
refKind = rewrittenReceiver.GetRefKind();

if (refKind == RefKind.None &&
!rewrittenReceiver.Type.IsReferenceType &&
Copy link
Member

Choose a reason for hiding this comment

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

!rewrittenReceiver.Type.IsReferenceType &&

Is this always true now?

Binder.HasHome(rewrittenReceiver,
Binder.AddressKind.Constrained,
_factory.CurrentFunction,
peVerifyCompatEnabled: false,
stackLocalsOpt: null))
{
refKind = RefKind.Ref;
}
}
}

Expand Down
50 changes: 23 additions & 27 deletions src/Compilers/CSharp/Test/Emit2/CodeGen/CodeGenCallTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,36 +2137,32 @@ public DummyHandler(int literalLength, int formattedCount, IMoveable logger)
verifier.VerifyIL("Program.Call1<T>",
@"
{
// Code size 70 (0x46)
// Code size 61 (0x3d)
.maxstack 6
.locals init (T& V_0,
T V_1,
DummyHandler V_2)
.locals init (T V_0,
DummyHandler V_1)
IL_0000: ldarg.0
IL_0001: ldobj ""T""
IL_0006: stloc.1
IL_0007: ldloca.s V_1
IL_0009: stloc.0
IL_000a: ldloc.0
IL_000b: ldarg.0
IL_000c: call ""int Program.GetOffset<T>(ref T)""
IL_0011: ldloca.s V_2
IL_0013: ldc.i4.4
IL_0014: ldc.i4.1
IL_0015: ldloc.0
IL_0016: ldobj ""T""
IL_001b: box ""T""
IL_0020: call ""DummyHandler..ctor(int, int, IMoveable)""
IL_0025: ldloca.s V_2
IL_0027: ldstr ""log:""
IL_002c: call ""void DummyHandler.AppendLiteral(string)""
IL_0031: ldloca.s V_2
IL_0033: ldc.i4.0
IL_0034: call ""void DummyHandler.AppendFormatted<int>(int)""
IL_0039: ldloc.2
IL_003a: constrained. ""T""
IL_0040: callvirt ""void IMoveable.GetName(int, DummyHandler)""
IL_0045: ret
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: box ""T""
IL_000d: ldarg.0
IL_000e: call ""int Program.GetOffset<T>(ref T)""
IL_0013: ldloca.s V_1
IL_0015: ldc.i4.4
IL_0016: ldc.i4.1
IL_0017: ldloc.0
IL_0018: box ""T""
IL_001d: call ""DummyHandler..ctor(int, int, IMoveable)""
IL_0022: ldloca.s V_1
IL_0024: ldstr ""log:""
IL_0029: call ""void DummyHandler.AppendLiteral(string)""
IL_002e: ldloca.s V_1
IL_0030: ldc.i4.0
IL_0031: call ""void DummyHandler.AppendFormatted<int>(int)""
IL_0036: ldloc.1
IL_0037: callvirt ""void IMoveable.GetName(int, DummyHandler)""
IL_003c: ret
}
");

Expand Down
Loading