Skip to content

Commit

Permalink
Fix Function Pointer RefKind Display (#51223)
Browse files Browse the repository at this point in the history 
Fixes #51222
Fixes #51224
  • Loading branch information
@YairHalberstadt
YairHalberstadt authored Mar 17, 2021
1 parent c016e64 commit a6cfe5f
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 103 deletions.
102 changes: 64 additions & 38 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,18 +609,25 @@ void visitFunctionPointerSignature(IMethodSymbol symbol)

foreach (var param in symbol.Parameters)
{
param.Accept(this.NotFirstVisitor);
AddParameterRefKind(param.RefKind);

AddCustomModifiersIfRequired(param.RefCustomModifiers);

param.Type.Accept(this.NotFirstVisitor);

AddCustomModifiersIfRequired(param.CustomModifiers, leadingSpace: true, trailingSpace: false);

AddPunctuation(SyntaxKind.CommaToken);
AddSpace();
}

if (symbol.ReturnsByRef)
{
AddRefIfRequired();
AddRef();
}
else if (symbol.ReturnsByRefReadonly)
{
AddRefReadonlyIfRequired();
AddRefReadonly();
}

AddCustomModifiersIfRequired(symbol.RefCustomModifiers);
Expand Down Expand Up @@ -679,9 +686,13 @@ public override void VisitParameter(IParameterSymbol symbol)
// used on their own or in the context of methods.

var includeType = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeType);
var includeName = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeName)
&& !(symbol.ContainingSymbol is IMethodSymbol { MethodKind: MethodKind.FunctionPointerSignature });
var includeName = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeName) &&
symbol.Name.Length != 0;
var includeBrackets = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeOptionalBrackets);
var includeDefaultValue = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeDefaultValue) &&
format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeName) &&
symbol.HasExplicitDefaultValue &&
CanAddConstant(symbol.Type, symbol.ExplicitDefaultValue);

if (includeBrackets && symbol.IsOptional)
{
Expand All @@ -703,26 +714,26 @@ public override void VisitParameter(IParameterSymbol symbol)
AddCustomModifiersIfRequired(symbol.CustomModifiers, leadingSpace: true, trailingSpace: false);
}

if (includeName && includeType)
{
AddSpace();
}

if (includeName)
{
if (includeType)
{
AddSpace();
}
var kind = symbol.IsThis ? SymbolDisplayPartKind.Keyword : SymbolDisplayPartKind.ParameterName;
builder.Add(CreatePart(kind, symbol, symbol.Name));
}

if (format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeDefaultValue) &&
symbol.HasExplicitDefaultValue &&
CanAddConstant(symbol.Type, symbol.ExplicitDefaultValue))
if (includeDefaultValue)
{
if (includeName || includeType)
{
AddSpace();
AddPunctuation(SyntaxKind.EqualsToken);
AddSpace();

AddConstantValue(symbol.Type, symbol.ExplicitDefaultValue);
}
AddPunctuation(SyntaxKind.EqualsToken);
AddSpace();

AddConstantValue(symbol.Type, symbol.ExplicitDefaultValue);
}

if (includeBrackets && symbol.IsOptional)
Expand Down Expand Up @@ -936,22 +947,32 @@ private void AddRefIfRequired()
{
if (format.MemberOptions.IncludesOption(SymbolDisplayMemberOptions.IncludeRef))
{
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
AddRef();
}
}

private void AddRef()
{
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
}

private void AddRefReadonlyIfRequired()
{
if (format.MemberOptions.IncludesOption(SymbolDisplayMemberOptions.IncludeRef))
{
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
AddKeyword(SyntaxKind.ReadOnlyKeyword);
AddSpace();
AddRefReadonly();
}
}

private void AddRefReadonly()
{
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
AddKeyword(SyntaxKind.ReadOnlyKeyword);
AddSpace();
}

private void AddReadOnlyIfRequired()
{
// 'readonly' in this context is effectively a 'ref' modifier
Expand All @@ -967,21 +988,26 @@ private void AddParameterRefKindIfRequired(RefKind refKind)
{
if (format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeParamsRefOut))
{
switch (refKind)
{
case RefKind.Out:
AddKeyword(SyntaxKind.OutKeyword);
AddSpace();
break;
case RefKind.Ref:
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
break;
case RefKind.In:
AddKeyword(SyntaxKind.InKeyword);
AddSpace();
break;
}
AddParameterRefKind(refKind);
}
}

private void AddParameterRefKind(RefKind refKind)
{
switch (refKind)
{
case RefKind.Out:
AddKeyword(SyntaxKind.OutKeyword);
AddSpace();
break;
case RefKind.Ref:
AddKeyword(SyntaxKind.RefKeyword);
AddSpace();
break;
case RefKind.In:
AddKeyword(SyntaxKind.InKeyword);
AddSpace();
break;
}
}
}
Expand Down
54 changes: 27 additions & 27 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ void M(C c)
var comp = CreateCompilationWithFunctionPointersAndIl(source, il);

comp.VerifyDiagnostics(
// (6,26): error CS0570: 'delegate*<int>' is not supported by the language
// (6,26): error CS0570: 'delegate*<ref int>' is not supported by the language
// ref int i1 = ref c.Field1();
Diagnostic(ErrorCode.ERR_BindToBogus, "c.Field1()").WithArguments("delegate*<int>").WithLocation(6, 26),
Diagnostic(ErrorCode.ERR_BindToBogus, "c.Field1()").WithArguments("delegate*<ref int>").WithLocation(6, 26),
// (6,28): error CS0570: 'C.Field1' is not supported by the language
// ref int i1 = ref c.Field1();
Diagnostic(ErrorCode.ERR_BindToBogus, "Field1").WithArguments("C.Field1").WithLocation(6, 28),
// (7,26): error CS0570: 'delegate*<int>' is not supported by the language
// (7,26): error CS0570: 'delegate*<ref int>' is not supported by the language
// ref int i2 = ref c.Field2();
Diagnostic(ErrorCode.ERR_BindToBogus, "c.Field2()").WithArguments("delegate*<int>").WithLocation(7, 26),
Diagnostic(ErrorCode.ERR_BindToBogus, "c.Field2()").WithArguments("delegate*<ref int>").WithLocation(7, 26),
// (7,28): error CS0570: 'C.Field2' is not supported by the language
// ref int i2 = ref c.Field2();
Diagnostic(ErrorCode.ERR_BindToBogus, "Field2").WithArguments("C.Field2").WithLocation(7, 28),
Expand Down Expand Up @@ -3223,18 +3223,18 @@ void M()
// (26,36): error CS8758: Ref mismatch between 'C.M6()' and function pointer 'delegate*<object>'
// delegate*<object> ptr14 = &M6;
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M6").WithArguments("C.M6()", "delegate*<object>").WithLocation(26, 36),
// (27,40): error CS8758: Ref mismatch between 'C.M6()' and function pointer 'delegate*<object>'
// (27,40): error CS8758: Ref mismatch between 'C.M6()' and function pointer 'delegate*<ref object>'
// delegate*<ref object> ptr15 = &M6;
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M6").WithArguments("C.M6()", "delegate*<object>").WithLocation(27, 40),
// (28,40): error CS8758: Ref mismatch between 'C.M7()' and function pointer 'delegate*<object>'
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M6").WithArguments("C.M6()", "delegate*<ref object>").WithLocation(27, 40),
// (28,40): error CS8758: Ref mismatch between 'C.M7()' and function pointer 'delegate*<ref object>'
// delegate*<ref object> ptr16 = &M7;
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M7").WithArguments("C.M7()", "delegate*<object>").WithLocation(28, 40),
// (29,49): error CS8758: Ref mismatch between 'C.M5()' and function pointer 'delegate*<object>'
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M7").WithArguments("C.M7()", "delegate*<ref object>").WithLocation(28, 40),
// (29,49): error CS8758: Ref mismatch between 'C.M5()' and function pointer 'delegate*<ref readonly object>'
// delegate*<ref readonly object> ptr17 = &M5;
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M5").WithArguments("C.M5()", "delegate*<object>").WithLocation(29, 49),
// (30,49): error CS8758: Ref mismatch between 'C.M7()' and function pointer 'delegate*<object>'
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M5").WithArguments("C.M5()", "delegate*<ref readonly object>").WithLocation(29, 49),
// (30,49): error CS8758: Ref mismatch between 'C.M7()' and function pointer 'delegate*<ref readonly object>'
// delegate*<ref readonly object> ptr18 = &M7;
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M7").WithArguments("C.M7()", "delegate*<object>").WithLocation(30, 49)
Diagnostic(ErrorCode.ERR_FuncPtrRefMismatch, "M7").WithArguments("C.M7()", "delegate*<ref readonly object>").WithLocation(30, 49)
);
}

Expand Down Expand Up @@ -5314,7 +5314,7 @@ void M<T>(delegate*<ref T> ptr1, delegate*<T> ptr2) where T : C
}
}");

verifier.VerifyIL(@"C.M<T>(delegate*<T>, delegate*<T>)", @"
verifier.VerifyIL(@"C.M<T>(delegate*<ref T>, delegate*<T>)", @"
{
// Code size 34 (0x22)
.maxstack 1
Expand Down Expand Up @@ -6321,10 +6321,10 @@ protected override void M1(delegate*<{refKind2}string> ptr) {{}}
comp.VerifyDiagnostics(
// (9,29): error CS0115: 'Derived.M1(delegate*<{refKind2} string>)': no suitable method found to override
// protected override void M1(delegate*<{refKind2} string> ptr) {}
Diagnostic(ErrorCode.ERR_OverrideNotExpected, "M1").WithArguments($"Derived.M1(delegate*<string>)").WithLocation(9, 29),
Diagnostic(ErrorCode.ERR_OverrideNotExpected, "M1").WithArguments($"Derived.M1(delegate*<{(string.IsNullOrWhiteSpace(refKind2) ? "" : refKind2)}string>)").WithLocation(9, 29),
// (10,49): error CS0508: 'Derived.M2()': return type must be 'delegate*<{refKind1} string>' to match overridden member 'Base.M2()'
// protected override delegate*<{refKind2} string> M2() => throw null;
Diagnostic(ErrorCode.ERR_CantChangeReturnTypeOnOverride, "M2").WithArguments("Derived.M2()", "Base.M2()", $"delegate*<string>").WithLocation(10, 42 + refKind2.Length)
Diagnostic(ErrorCode.ERR_CantChangeReturnTypeOnOverride, "M2").WithArguments("Derived.M2()", "Base.M2()", $"delegate*<{(string.IsNullOrWhiteSpace(refKind1) ? "" : refKind1)}string>").WithLocation(10, 42 + refKind2.Length)
);
}

Expand Down Expand Up @@ -7078,12 +7078,12 @@ static void M2(delegate*<ref string, ref string> ptr1)
// (18,32): warning CS8619: Nullability of reference types in value of type 'string' doesn't match target type 'string?'.
// ref string? str3 = ref ptr1(ref str2);
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "ptr1(ref str2)").WithArguments("string", "string?").WithLocation(18, 32),
// (19,51): warning CS8619: Nullability of reference types in value of type 'delegate*<ref string, string>' doesn't match target type 'delegate*<ref string?, string>'.
// (19,51): warning CS8619: Nullability of reference types in value of type 'delegate*<ref string, ref string>' doesn't match target type 'delegate*<ref string?, ref string>'.
// delegate*<ref string?, ref string> ptr2 = ptr1;
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "ptr1").WithArguments("delegate*<ref string, string>", "delegate*<ref string?, string>").WithLocation(19, 51),
// (20,51): warning CS8619: Nullability of reference types in value of type 'delegate*<ref string, string>' doesn't match target type 'delegate*<ref string, string?>'.
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "ptr1").WithArguments("delegate*<ref string, ref string>", "delegate*<ref string?, ref string>").WithLocation(19, 51),
// (20,51): warning CS8619: Nullability of reference types in value of type 'delegate*<ref string, ref string>' doesn't match target type 'delegate*<ref string, ref string?>'.
// delegate*<ref string, ref string?> ptr3 = ptr1;
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "ptr1").WithArguments("delegate*<ref string, string>", "delegate*<ref string, string?>").WithLocation(20, 51)
Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "ptr1").WithArguments("delegate*<ref string, ref string>", "delegate*<ref string, ref string?>").WithLocation(20, 51)
);
}

Expand Down Expand Up @@ -10775,7 +10775,7 @@ static ref int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int i)
static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int)", @"
verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, ref int>, ref int)", @"
{
// Code size 10 (0xa)
.maxstack 2
Expand Down Expand Up @@ -10831,9 +10831,9 @@ static ref Span<int> ReturnPtrByRef(delegate*<ref Span<int>, ref Span<int>> ptr)
}", options: TestOptions.UnsafeReleaseExe);

comp.VerifyDiagnostics(
// (10,20): error CS8347: Cannot use a result of 'delegate*<ref Span<int>, Span<int>>' in this context because it may expose variables referenced by parameter '0' outside of their declaration scope
// (10,20): error CS8347: Cannot use a result of 'delegate*<ref Span<int>, ref Span<int>>' in this context because it may expose variables referenced by parameter '0' outside of their declaration scope
// return ref ptr(ref span);
Diagnostic(ErrorCode.ERR_EscapeCall, "ptr(ref span)").WithArguments("delegate*<ref System.Span<int>, System.Span<int>>", "0").WithLocation(10, 20),
Diagnostic(ErrorCode.ERR_EscapeCall, "ptr(ref span)").WithArguments("delegate*<ref System.Span<int>, ref System.Span<int>>", "0").WithLocation(10, 20),
// (10,28): error CS8168: Cannot return local 'span' by reference because it is not a ref local
// return ref ptr(ref span);
Diagnostic(ErrorCode.ERR_RefReturnLocal, "span").WithArguments("span").WithLocation(10, 28)
Expand Down Expand Up @@ -10861,7 +10861,7 @@ static ref Span<int> ReturnPtrByRef(delegate*<ref Span<int>, ref Span<int>> ptr,

var verifier = CompileAndVerify(comp, expectedOutput: "2", verify: Verification.Skipped);

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref System.Span<int>, System.Span<int>>, ref System.Span<int>)", @"
verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref System.Span<int>, ref System.Span<int>>, ref System.Span<int>)", @"
{
// Code size 10 (0xa)
.maxstack 2
Expand Down Expand Up @@ -10892,9 +10892,9 @@ static ref int ReturnPtrByRef(delegate*<ref int, ref readonly int> ptr, ref int
}", options: TestOptions.UnsafeReleaseExe);

comp.VerifyDiagnostics(
// (8,16): error CS8333: Cannot return method 'delegate*<ref int, int>' by writable reference because it is a readonly variable
// (8,16): error CS8333: Cannot return method 'delegate*<ref int, ref readonly int>' by writable reference because it is a readonly variable
// => ref ptr(ref i);
Diagnostic(ErrorCode.ERR_RefReturnReadonlyNotField, "ptr(ref i)").WithArguments("method", "delegate*<ref int, int>").WithLocation(8, 16)
Diagnostic(ErrorCode.ERR_RefReturnReadonlyNotField, "ptr(ref i)").WithArguments("method", "delegate*<ref int, ref readonly int>").WithLocation(8, 16)
);
}

Expand All @@ -10915,7 +10915,7 @@ static ref readonly int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int
static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int)", @"
verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, ref int>, ref int)", @"
{
// Code size 10 (0xa)
.maxstack 2
Expand Down Expand Up @@ -11023,7 +11023,7 @@ static ref int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int i, ref in
static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int, ref int)", @"
verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, ref int>, ref int, ref int)", @"
{
// Code size 10 (0xa)
.maxstack 2
Expand Down
Loading

0 comments on commit a6cfe5f

Please sign in to comment.