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

Handle ref returning receivers in dynamic calls #70676

Merged
merged 6 commits into from
Nov 9, 2023
Merged

Handle ref returning receivers in dynamic calls #70676

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10deff8
Handle ref returning receivers in dynamic calls
jjonescz Nov 2, 2023
049704f
Add more tests
jjonescz Nov 6, 2023
2b6e2d3
Use `HasHome` helper to decide dynamic receiver ref kind
jjonescz Nov 6, 2023
0c7d3bd
Fix desktop tests
jjonescz Nov 6, 2023
1381cff
Simplify
jjonescz Nov 6, 2023
16608ce
Merge branch 'main' into 68063-DynamicOpFactoryRefKind
jjonescz Nov 9, 2023
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
29 changes: 7 additions & 22 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LoweredDynamicOperationFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,35 +539,20 @@ private MethodSymbol GetArgumentInfoFactory()
// however
// dynamic d = ...;
// GetS().M(d); // becomes Site(GetS(), d) without ref on the target obj arg
internal static RefKind GetReceiverRefKind(BoundExpression loweredReceiver)
internal RefKind GetReceiverRefKind(BoundExpression loweredReceiver)
{
Debug.Assert(loweredReceiver.Type is { });
if (!loweredReceiver.Type.IsValueType)
{
return RefKind.None;
}

switch (loweredReceiver.Kind)
{
case BoundKind.Parameter:
Debug.Assert(!LocalRewriter.IsCapturedPrimaryConstructorParameter(loweredReceiver));
goto case BoundKind.Local;

case BoundKind.Local:
case BoundKind.ArrayAccess:
case BoundKind.ThisReference:
case BoundKind.PointerIndirectionOperator:
case BoundKind.PointerElementAccess:
case BoundKind.RefValueOperator:
return RefKind.Ref;

case BoundKind.BaseReference:
// base dynamic dispatch is not supported, an error has already been reported
case BoundKind.TypeExpression:
throw ExceptionUtilities.UnexpectedValue(loweredReceiver.Kind);
}

return RefKind.None;
var hasHome = Binder.HasHome(loweredReceiver,
Binder.AddressKind.Writeable,
_factory.CurrentFunction,
_factory.Compilation.IsPeVerifyCompatEnabled,
Copy link
Contributor

@AlekseyTs AlekseyTs Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

_factory.Compilation.IsPeVerifyCompatEnabled

I think we don't need to depend on this setting here. The most permissive value is probably going to be the closest match with the previous behavior. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, at least, with intent.

stackLocalsOpt: null);
return hasHome ? RefKind.Ref : RefKind.None;
}

internal BoundExpression MakeCallSiteArgumentInfos(
Expand Down
48 changes: 24 additions & 24 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDynamicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9235,10 +9235,10 @@ public void goo(int a) {}
";
CompileAndVerifyIL(source, "C.M", @"
{
// Code size 97 (0x61)
// Code size 98 (0x62)
.maxstack 9
IL_0000: ldsfld ""System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0005: brtrue.s IL_0045
IL_0000: ldsfld ""System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0005: brtrue.s IL_0046
IL_0007: ldc.i4 0x100
IL_000c: ldstr ""goo""
IL_0011: ldnull
Expand All @@ -9248,27 +9248,27 @@ .maxstack 9
IL_001d: newarr ""Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo""
IL_0022: dup
IL_0023: ldc.i4.0
IL_0024: ldc.i4.1
IL_0025: ldnull
IL_0026: call ""Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)""
IL_002b: stelem.ref
IL_002c: dup
IL_002d: ldc.i4.1
IL_002e: ldc.i4.0
IL_002f: ldnull
IL_0030: call ""Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)""
IL_0035: stelem.ref
IL_0036: call ""System.Runtime.CompilerServices.CallSiteBinder Microsoft.CSharp.RuntimeBinder.Binder.InvokeMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags, string, System.Collections.Generic.IEnumerable<System.Type>, System.Type, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>)""
IL_003b: call ""System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>> System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>>.Create(System.Runtime.CompilerServices.CallSiteBinder)""
IL_0040: stsfld ""System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0045: ldsfld ""System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_004a: ldfld ""System.Action<System.Runtime.CompilerServices.CallSite, S, object> System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>>.Target""
IL_004f: ldsfld ""System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0054: ldarg.0
IL_0055: ldfld ""S C.s""
IL_005a: ldarg.1
IL_005b: callvirt ""void System.Action<System.Runtime.CompilerServices.CallSite, S, object>.Invoke(System.Runtime.CompilerServices.CallSite, S, object)""
IL_0060: ret
IL_0024: ldc.i4.s 9
IL_0026: ldnull
IL_0027: call ""Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)""
IL_002c: stelem.ref
IL_002d: dup
IL_002e: ldc.i4.1
IL_002f: ldc.i4.0
IL_0030: ldnull
IL_0031: call ""Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)""
IL_0036: stelem.ref
IL_0037: call ""System.Runtime.CompilerServices.CallSiteBinder Microsoft.CSharp.RuntimeBinder.Binder.InvokeMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags, string, System.Collections.Generic.IEnumerable<System.Type>, System.Type, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>)""
IL_003c: call ""System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>> System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>>.Create(System.Runtime.CompilerServices.CallSiteBinder)""
IL_0041: stsfld ""System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0046: ldsfld ""System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_004b: ldfld ""<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object> System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>>.Target""
IL_0050: ldsfld ""System.Runtime.CompilerServices.CallSite<<>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>> C.<>o__1.<>p__0""
IL_0055: ldarg.0
IL_0056: ldflda ""S C.s""
IL_005b: ldarg.1
IL_005c: callvirt ""void <>A{00000008}<System.Runtime.CompilerServices.CallSite, S, object>.Invoke(System.Runtime.CompilerServices.CallSite, ref S, object)""
IL_0061: ret
}
");
}
Expand Down
211 changes: 211 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/DynamicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1510,6 +1510,217 @@ public void Goo()
VerifyOperationTreeAndDiagnosticsForTest<InvocationExpressionSyntax>(source, expectedOperationTree, expectedDiagnostics);
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void RefReturn_Method([CombinatorialValues("int", "dynamic")] string type)
{
var source = $$"""
{{type}} d = 1;
S s = default;
GetS(ref s).M(d);
System.Console.Write(s.X);

static ref S GetS(ref S s) => ref s;

struct S
{
public int X { get; private set; }

public void M(int x) => X = x;
}
""";
CompileAndVerify(source, new[] { CSharpRef },
expectedOutput: "1", verify: Verification.Fails).VerifyDiagnostics();
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void RefReturn_Property([CombinatorialValues("int", "dynamic")] string type)
{
var source = $$"""
{{type}} d = 1;
S s = default;
s.Self.M(d);
System.Console.Write(s.X);

struct S
{
[System.Diagnostics.CodeAnalysis.UnscopedRef]
public ref S Self => ref this;
public int X { get; private set; }

public void M(int x) => X = x;
}
""";
CompileAndVerify(new[] { source, UnscopedRefAttributeDefinition }, new[] { CSharpRef },
expectedOutput: "1", verify: Verification.Fails).VerifyDiagnostics();
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void RefReturn_Indexer([CombinatorialValues("int", "dynamic")] string type)
{
var source = $$"""
{{type}} d = 1;
S s = default;
s[0].M(d);
System.Console.Write(s.X);

struct S
{
[System.Diagnostics.CodeAnalysis.UnscopedRef]
public ref S this[int _] => ref this;
public int X { get; private set; }

public void M(int x) => X = x;
}
""";
CompileAndVerify(new[] { source, UnscopedRefAttributeDefinition }, new[] { CSharpRef },
expectedOutput: "1", verify: Verification.Fails).VerifyDiagnostics();
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void RefReturn_Local([CombinatorialValues("int", "dynamic")] string type)
{
var source = $$"""
{{type}} d = 1;
S s = default;
ref S r = ref s;
r.M(d);
System.Console.Write(s.X);

struct S
{
public int X { get; private set; }
public void M(int x) => X = x;
}
""";
CompileAndVerify(new[] { source, UnscopedRefAttributeDefinition }, new[] { CSharpRef },
expectedOutput: "1").VerifyDiagnostics();
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void RefReturn_Field([CombinatorialValues("int", "dynamic")] string type)
{
var source = $$"""
{{type}} d = 1;
S s = default;
P p = new P(ref s);
p.S.M(d);
System.Console.Write(s.X);

ref struct P(ref S s)
{
public ref S S = ref s;
}

struct S
{
public int X { get; private set; }
public void M(int x) => X = x;
}
""";
CompileAndVerify(new[] { source, UnscopedRefAttributeDefinition }, targetFramework: TargetFramework.Net70,
expectedOutput: RefFieldTests.IncludeExpectedOutput("1"), verify: Verification.FailsPEVerify).VerifyDiagnostics();
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void StructReceiver_Field([CombinatorialValues("int", "dynamic")] string type, bool ro)
{
var source = $$"""
new C(default).Run();

class C(S s)
{
private {{(ro ? "readonly" : "")}} S s = s;

public void Run()
{
{{type}} d = 1;
s.M(d);
System.Console.Write(s.X);
}
}

struct S
{
public int X { get; private set; }
public void M(int x) => X = x;
}
""";
CompileAndVerify(new[] { source, UnscopedRefAttributeDefinition }, new[] { CSharpRef },
expectedOutput: ro ? "0" : "1", verify: ro ? Verification.FailsPEVerify : default).VerifyDiagnostics();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/68063")]
public void StructReceiver_Rvalue()
{
var source = """
class Program
{
static void Main()
{
dynamic d = 1;
GetS().M(d);
System.Console.Write(GetS().X);
Copy link
Contributor

@AlekseyTs AlekseyTs Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

System.Console.Write(GetS().X);

It looks like this line isn't meaningful. It is guaranteed to produce the same output regardless of what the previous line is doing. #Closed

}

static S GetS() => default;
}

struct S
{
public int X { get; private set; }
public void M(int x) => X = x;
}
""";
var verifier = CompileAndVerify(source, new[] { CSharpRef }, expectedOutput: "0").VerifyDiagnostics();
verifier.VerifyIL("Program.Main", """
{
// Code size 121 (0x79)
.maxstack 9
.locals init (object V_0, //d
S V_1)
IL_0000: ldc.i4.1
IL_0001: box "int"
IL_0006: stloc.0
IL_0007: ldsfld "System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>> Program.<>o__0.<>p__0"
IL_000c: brtrue.s IL_004c
IL_000e: ldc.i4 0x100
IL_0013: ldstr "M"
IL_0018: ldnull
IL_0019: ldtoken "Program"
IL_001e: call "System.Type System.Type.GetTypeFromHandle(System.RuntimeTypeHandle)"
IL_0023: ldc.i4.2
IL_0024: newarr "Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo"
IL_0029: dup
IL_002a: ldc.i4.0
IL_002b: ldc.i4.1
IL_002c: ldnull
IL_002d: call "Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)"
IL_0032: stelem.ref
IL_0033: dup
IL_0034: ldc.i4.1
IL_0035: ldc.i4.0
IL_0036: ldnull
IL_0037: call "Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo.Create(Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags, string)"
IL_003c: stelem.ref
IL_003d: call "System.Runtime.CompilerServices.CallSiteBinder Microsoft.CSharp.RuntimeBinder.Binder.InvokeMember(Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags, string, System.Collections.Generic.IEnumerable<System.Type>, System.Type, System.Collections.Generic.IEnumerable<Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo>)"
IL_0042: call "System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>> System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>>.Create(System.Runtime.CompilerServices.CallSiteBinder)"
IL_0047: stsfld "System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>> Program.<>o__0.<>p__0"
IL_004c: ldsfld "System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>> Program.<>o__0.<>p__0"
IL_0051: ldfld "System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic> System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>>.Target"
IL_0056: ldsfld "System.Runtime.CompilerServices.CallSite<System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>> Program.<>o__0.<>p__0"
IL_005b: call "S Program.GetS()"
IL_0060: ldloc.0
IL_0061: callvirt "void System.Action<System.Runtime.CompilerServices.CallSite, S, dynamic>.Invoke(System.Runtime.CompilerServices.CallSite, S, dynamic)"
IL_0066: call "S Program.GetS()"
IL_006b: stloc.1
IL_006c: ldloca.s V_1
IL_006e: call "readonly int S.X.get"
IL_0073: call "void System.Console.Write(int)"
IL_0078: ret
}
""");
}

#endregion

#region Type Inference
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public class RefFieldTests : CSharpTestBase
{
private static string IncludeExpectedOutput(string expectedOutput) => ExecutionConditionUtil.IsMonoOrCoreClr ? expectedOutput : null;
internal static string IncludeExpectedOutput(string expectedOutput) => ExecutionConditionUtil.IsMonoOrCoreClr ? expectedOutput : null;

[CombinatorialData]
[Theory]
Expand Down
Loading