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

Fix intercepting a struct or enum base method #71655

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Fix intercepting a struct or enum base method #71655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b40359a
Fix intercepting a struct or enum base method
RikkiGibson Jan 16, 2024
c8c47e8
Address feedback
RikkiGibson Jan 19, 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
6 changes: 5 additions & 1 deletion 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 @@ -272,7 +272,11 @@ private void InterceptCallAndAdjustArguments(
if (needToReduce)
{
Debug.Assert(methodThisParameter is not null);
arguments = arguments.Insert(0, receiverOpt!);
Debug.Assert(receiverOpt?.Type is not null);
Debug.Assert(receiverOpt.Type.Equals(interceptor.Parameters[0].Type, TypeCompareKind.AllIgnoreOptions)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try to add a clarifying comment here in a follow-up PR.

|| (!receiverOpt.Type.IsReferenceType && interceptor.Parameters[0].Type.IsReferenceType));
receiverOpt = MakeConversionNode(receiverOpt, interceptor.Parameters[0].Type, @checked: false, markAsChecked: true);
arguments = arguments.Insert(0, receiverOpt);
receiverOpt = null;

// CodeGenerator.EmitArguments requires that we have a fully-filled-out argumentRefKindsOpt for any ref/in/out arguments.
Expand Down
367 changes: 367 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/InterceptorsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3423,6 +3423,112 @@ static class D
Diagnostic(ErrorCode.ERR_InterceptorSignatureMismatch, @"InterceptsLocation(""Program.cs"", 11, 9)").WithArguments("Program.InterceptableMethod(ref readonly int)", "D.Interceptor(in int)").WithLocation(17, 6));
}

[Fact]
public void SignatureMismatch_10()
{
var source = """
using System.Runtime.CompilerServices;
using System;

struct Program
{
public void InterceptableMethod() => Console.Write("Original");

public static void Main()
{
new Program().InterceptableMethod();
}
}

static class D
{
[InterceptsLocation("Program.cs", 10, 23)]
public static void Interceptor(this in Program x) => Console.Write("Intercepted");
}
""";
var comp = CreateCompilation(new[] { (source, "Program.cs"), s_attributesSource }, parseOptions: RegularWithInterceptors);
comp.VerifyEmitDiagnostics(
// Program.cs(16,6): error CS9148: Interceptor must have a 'this' parameter matching parameter 'ref Program this' on 'Program.InterceptableMethod()'.
// [InterceptsLocation("Program.cs", 10, 23)]
Diagnostic(ErrorCode.ERR_InterceptorMustHaveMatchingThisParameter, @"InterceptsLocation(""Program.cs"", 10, 23)").WithArguments("ref Program this", "Program.InterceptableMethod()").WithLocation(16, 6));
}
Copy link
Member

Choose a reason for hiding this comment

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

}

Consider testing readonly void InterceptableMethod() with static void Interceptor(this in Program), static void Interceptor(this ref readonly Program), and static void Interceptor(this ref Program).


[Fact]
public void SignatureMismatch_11()
{
var source = ("""
using System;

struct Program
{
public readonly void InterceptableMethod() => Console.Write("Original");

public static void Main()
{
new Program().InterceptableMethod();
}
}
""", "Program.cs");

var interceptor = ("""
using System.Runtime.CompilerServices;
using System;

static class D
{
[InterceptsLocation("Program.cs", 9, 23)]
public static void Interceptor(this in Program x) => Console.Write("Intercepted");
}
""", "Interceptor.cs");
var verifier = CompileAndVerify(new[] { source, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Original");
verifier.VerifyDiagnostics();

verifier = CompileAndVerify(new[] { source, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Intercepted");
verifier.VerifyDiagnostics();
}

[Theory]
[InlineData("ref readonly")]
[InlineData("ref")]
[WorkItem("https://github.com/dotnet/roslyn/issues/71714")]
public void SignatureMismatch_12(string interceptorRefKind)
{
var source = ("""
using System;

struct Program
{
public readonly void InterceptableMethod() => Console.Write("Original");

public static void Main()
{
new Program().InterceptableMethod();
}
}
""", "Program.cs");

var interceptor = ($$"""
using System.Runtime.CompilerServices;
using System;

static class D
{
[InterceptsLocation("Program.cs", 9, 23)]
public static void Interceptor(this {{interceptorRefKind}} Program x) => Console.Write("Intercepted");
}
""", "Interceptor.cs");
var verifier = CompileAndVerify(new[] { source, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Original");
verifier.VerifyDiagnostics();

// 'this ref readonly' should probably be compatible with 'readonly' original method.
// Tracked by https://github.com/dotnet/roslyn/issues/71714
var comp = CreateCompilation(new[] { source, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors);
comp.VerifyEmitDiagnostics(
// Interceptor.cs(6,6): error CS9148: Interceptor must have a 'this' parameter matching parameter 'in Program this' on 'Program.InterceptableMethod()'.
// [InterceptsLocation("Program.cs", 9, 23)]
Diagnostic(ErrorCode.ERR_InterceptorMustHaveMatchingThisParameter, @"InterceptsLocation(""Program.cs"", 9, 23)").WithArguments("in Program this", "Program.InterceptableMethod()").WithLocation(6, 6));
}

[Fact]
public void ScopedMismatch_01()
{
Expand Down Expand Up @@ -4827,4 +4933,265 @@ public static void Interceptor() { }
// [InterceptsLocation("Program.cs", 5, 3)]
Diagnostic(ErrorCode.ERR_InterceptorCannotUseUnmanagedCallersOnly, @"InterceptsLocation(""Program.cs"", 5, 3)").WithLocation(14, 6));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70841")]
public void InterceptorEnumBaseMethod()
{
var program = ("""
using System;

var value = MyEnum.Second;
Console.WriteLine(value.ToString());

public enum MyEnum
{
First,
Second,
}
""", "Program.cs");

var interceptor = ("""
using System.Runtime.CompilerServices;

namespace MyInterceptors
{
public static class Interceptors
{
[InterceptsLocation(@"Program.cs", 4, 25)]
public static string OtherToString(this System.Enum value)
=> "Wrong Value" + value;
}
}
""", "Interceptor.cs");

var verifier = CompileAndVerify(new[] { program, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Second");
verifier.VerifyDiagnostics();
verifier.VerifyIL("<top-level-statements-entry-point>", """
{
// Code size 21 (0x15)
.maxstack 1
.locals init (MyEnum V_0) //value
IL_0000: ldc.i4.1
IL_0001: stloc.0
IL_0002: ldloca.s V_0
IL_0004: constrained. "MyEnum"
IL_000a: callvirt "string object.ToString()"
IL_000f: call "void System.Console.WriteLine(string)"
IL_0014: ret
}
""");

verifier = CompileAndVerify(new[] { program, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Wrong ValueSecond");
verifier.VerifyDiagnostics();
verifier.VerifyIL("<top-level-statements-entry-point>", """
{
// Code size 17 (0x11)
.maxstack 1
IL_0000: ldc.i4.1
IL_0001: box "MyEnum"
IL_0006: call "string MyInterceptors.Interceptors.OtherToString(System.Enum)"
IL_000b: call "void System.Console.WriteLine(string)"
IL_0010: ret
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70841")]
public void InterceptorStructBaseMethod()
{
var program = ("""
using System;

MyStruct value = default;
Console.WriteLine(value.Equals((object)1));

public struct MyStruct { }
""", "Program.cs");

var interceptor = ("""
using System.Runtime.CompilerServices;

namespace MyInterceptors
{
public static class Interceptors
{
[InterceptsLocation(@"Program.cs", 4, 25)]
public static bool Equals(this System.ValueType value, object other) => true;
}
}
""", "Interceptor.cs");

var verifier = CompileAndVerify(new[] { program, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "False");
verifier.VerifyDiagnostics();
verifier.VerifyIL("<top-level-statements-entry-point>", """
{
// Code size 33 (0x21)
.maxstack 2
.locals init (MyStruct V_0) //value
IL_0000: ldloca.s V_0
IL_0002: initobj "MyStruct"
IL_0008: ldloca.s V_0
IL_000a: ldc.i4.1
IL_000b: box "int"
IL_0010: constrained. "MyStruct"
IL_0016: callvirt "bool object.Equals(object)"
IL_001b: call "void System.Console.WriteLine(bool)"
IL_0020: ret
}
""");

verifier = CompileAndVerify(new[] { program, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "True");
verifier.VerifyDiagnostics();
verifier.VerifyIL("<top-level-statements-entry-point>", """
{
// Code size 31 (0x1f)
.maxstack 2
.locals init (MyStruct V_0)
IL_0000: ldloca.s V_0
IL_0002: initobj "MyStruct"
IL_0008: ldloc.0
IL_0009: box "MyStruct"
IL_000e: ldc.i4.1
IL_000f: box "int"
IL_0014: call "bool MyInterceptors.Interceptors.Equals(System.ValueType, object)"
IL_0019: call "void System.Console.WriteLine(bool)"
IL_001e: ret
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70841")]
public void InterceptorTypeParameterObjectMethod()
{
var program = ("""
using System;

M("a");
void M<T>(T value)
{
Console.WriteLine(value.Equals((object)1));
}

public struct MyStruct { }
""", "Program.cs");

var interceptor = ("""
using System.Runtime.CompilerServices;

namespace MyInterceptors
{
public static class Interceptors
{
[InterceptsLocation(@"Program.cs", 6, 29)]
public static new bool Equals(this object value, object other) => true;
}
}
""", "Interceptor.cs");

var verifier = CompileAndVerify(new[] { program, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "False");
verifier.VerifyDiagnostics();
verifier.VerifyIL("Program.<<Main>$>g__M|0_0<T>(T)", """
{
// Code size 25 (0x19)
.maxstack 2
IL_0000: ldarga.s V_0
IL_0002: ldc.i4.1
IL_0003: box "int"
IL_0008: constrained. "T"
IL_000e: callvirt "bool object.Equals(object)"
IL_0013: call "void System.Console.WriteLine(bool)"
IL_0018: ret
}
""");

verifier = CompileAndVerify(new[] { program, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "True");
verifier.VerifyDiagnostics();
verifier.VerifyIL("Program.<<Main>$>g__M|0_0<T>(T)", """
{
// Code size 23 (0x17)
.maxstack 2
IL_0000: ldarg.0
IL_0001: box "T"
IL_0006: ldc.i4.1
IL_0007: box "int"
IL_000c: call "bool MyInterceptors.Interceptors.Equals(object, object)"
IL_0011: call "void System.Console.WriteLine(bool)"
IL_0016: ret
}
""");
}

[Theory]
[WorkItem("https://github.com/dotnet/roslyn/issues/70841")]
[InlineData("where T : struct, I")]
[InlineData("where T : I")]
public void InterceptorStructConstrainedInterfaceMethod(string constraints)
{
var program = ($$"""
using System;

C.M(default(MyStruct));

class C
{
public static void M<T>(T t) {{constraints}}
{
t.IM();
}
}

public struct MyStruct : I
{
public void IM()
{
Console.Write("Original");
}
}

public interface I
{
void IM();
}
""", "Program.cs");

var interceptor = ("""
using System.Runtime.CompilerServices;
using System;

namespace MyInterceptors
{
public static class Interceptors
{
[InterceptsLocation(@"Program.cs", 9, 11)]
public static void IM(this I @this) { Console.Write("Interceptor"); }
}
}
""", "Interceptor.cs");

var verifier = CompileAndVerify(new[] { program, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Original");
verifier.VerifyDiagnostics();
verifier.VerifyIL("C.M<T>(T)", """
{
// Code size 14 (0xe)
.maxstack 1
IL_0000: ldarga.s V_0
IL_0002: constrained. "T"
IL_0008: callvirt "void I.IM()"
IL_000d: ret
}
""");

verifier = CompileAndVerify(new[] { program, interceptor, s_attributesSource }, parseOptions: RegularWithInterceptors, expectedOutput: "Interceptor");
verifier.VerifyDiagnostics();
verifier.VerifyIL("C.M<T>(T)", """
{
// Code size 12 (0xc)
.maxstack 1
IL_0000: ldarg.0
IL_0001: box "T"
IL_0006: call "void MyInterceptors.Interceptors.IM(I)"
IL_000b: ret
}
""");
}
}