From ed7daf1926f32479f3d873416dc57a379143920f Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 5 May 2020 17:51:02 -0700 Subject: [PATCH 01/11] Ensure custom mods and ref specifiers are printed. --- .../SymbolDisplayVisitor.Members.cs | 18 +++++++++++++ .../CodeGen/CodeGenFunctionPointersTests.cs | 27 ++++++++++--------- .../Semantics/FunctionPointerTests.cs | 7 +++-- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs index 11461e35c9fde..a4e171fd055b9 100644 --- a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs +++ b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs @@ -280,8 +280,26 @@ public override void VisitMethod(IMethodSymbol symbol) AddSpace(); } + if (ShouldMethodDisplayReadOnly(symbol)) + { + AddReadOnlyIfRequired(); + } + + if (symbol.ReturnsByRef) + { + AddRefIfRequired(); + } + else if (symbol.ReturnsByRefReadonly) + { + AddRefReadonlyIfRequired(); + } + + AddCustomModifiersIfRequired(symbol.RefCustomModifiers); + symbol.ReturnType.Accept(this.NotFirstVisitor); + AddCustomModifiersIfRequired(symbol.ReturnTypeCustomModifiers, leadingSpace: true, trailingSpace: false); + AddPunctuation(SyntaxKind.GreaterThanToken); return; } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index 89b86e0728453..c6f8ca2d9c492 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -397,9 +397,9 @@ static void TestIn() // Code size 87 (0x57) .maxstack 2 IL_0000: ldftn ""ref readonly int D.M()"" - IL_0006: stsfld ""delegate* C.Field1"" - IL_000b: ldsfld ""delegate* C.Field1"" - IL_0010: calli ""delegate*"" + IL_0006: stsfld ""delegate* C.Field1"" + IL_000b: ldsfld ""delegate* C.Field1"" + IL_0010: calli ""delegate*"" IL_0015: dup IL_0016: ldind.i4 IL_0017: call ""void System.Console.Write(int)"" @@ -408,11 +408,11 @@ .maxstack 2 IL_0022: ldind.i4 IL_0023: call ""void System.Console.Write(int)"" IL_0028: ldftn ""ref readonly int D.M()"" - IL_002e: stsfld ""delegate* C.Field2"" + IL_002e: stsfld ""delegate* C.Field2"" IL_0033: ldc.i4.3 IL_0034: stsfld ""int D.i"" - IL_0039: ldsfld ""delegate* C.Field2"" - IL_003e: calli ""delegate*"" + IL_0039: ldsfld ""delegate* C.Field2"" + IL_003e: calli ""delegate*"" IL_0043: dup IL_0044: ldind.i4 IL_0045: call ""void System.Console.Write(int)"" @@ -538,7 +538,7 @@ static void Main() // Code size 31 (0x1f) .maxstack 2 IL_0000: ldftn ""ref readonly int C.GetI()"" - IL_0006: calli ""delegate*"" + IL_0006: calli ""delegate*"" IL_000b: dup IL_000c: ldind.i4 IL_000d: call ""void System.Console.Write(int)"" @@ -2402,8 +2402,8 @@ static void Main() .maxstack 1 IL_0000: ldstr ""Field"" IL_0005: stsfld ""string Program.field"" - IL_000a: call ""delegate* Program.LoadPtr()"" - IL_000f: calli ""delegate*"" + IL_000a: call ""delegate* Program.LoadPtr()"" + IL_000f: calli ""delegate*"" IL_0014: ldind.ref IL_0015: call ""void System.Console.WriteLine(string)"" IL_001a: ret @@ -2465,14 +2465,15 @@ static void Main() { // Code size 27 (0x1b) .maxstack 2 - IL_0000: call ""delegate* Program.LoadPtr()"" - IL_0005: calli ""delegate*"" + IL_0000: call ""delegate* Program.LoadPtr()"" + IL_0005: calli ""delegate*"" IL_000a: ldstr ""Field"" IL_000f: stind.ref IL_0010: ldsfld ""string Program.field"" IL_0015: call ""void System.Console.WriteLine(string)"" IL_001a: ret -}"); +} +"); } [Fact] @@ -5061,7 +5062,7 @@ void M(delegate* ptr1, delegate* ptr2) where T : C // Code size 34 (0x22) .maxstack 1 IL_0000: ldarg.1 - IL_0001: calli ""delegate*"" + IL_0001: calli ""delegate*"" IL_0006: constrained. ""T"" IL_000c: callvirt ""void C.M1()"" IL_0011: ldarg.2 diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs index 6d9f8259ce9a2..e48bab5e6e89d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs @@ -1,4 +1,3 @@ - // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -497,7 +496,7 @@ void M(delegate* param1, delegate* par // Code size 10 (0xa) .maxstack 1 .locals init (delegate* V_0, //ptr1 - delegate* V_1, //ptr2 + delegate* V_1, //ptr2 delegate* V_2, //ptr3 delegate* V_3) //ptr4 IL_0000: ldarg.1 @@ -1411,11 +1410,11 @@ void M(bool b) "delegate*", "?", "?", - "delegate*", + "delegate*", "delegate*", "?", "?", - "delegate*" + "delegate*" }; AssertEx.Equal(expectedTypes, invocationTypes); From 38402cd7e6a72acbdf5df6f5b3689b51fd96c49a Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 5 May 2020 17:51:13 -0700 Subject: [PATCH 02/11] Support dynamic type encoding and decoding. --- .../Symbols/Compilation_WellKnownMembers.cs | 43 ++++ .../Symbols/MemberSymbolExtensions.cs | 1 + .../Symbols/Metadata/PE/DynamicTypeDecoder.cs | 75 ++++++ .../Portable/Symbols/TypeWithAnnotations.cs | 3 + .../Utilities/TypeSymbolExtensions.cs | 38 ++- .../CodeGen/CodeGenFunctionPointersTests.cs | 219 ++++++++++++++++++ 6 files changed, 375 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index f7bae061be49d..06648fb91768e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -887,6 +887,13 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsB transformFlagsBuilder.Add(false); break; + case TypeKind.FunctionPointer: + handleFunctionPointerType((FunctionPointerTypeSymbol)type, transformFlagsBuilder, isNestedNamedType, addCustomModifierFlags); + + // Function pointer types have nested custom modifiers and refkinds in line with types, and visit all their nested types + // as part of this call. + return true; + default: // Encode transforms flag for this type. // For nested named types, a single flag (false) is encoded for the entire type name, followed by flags for all of the type arguments. @@ -906,6 +913,42 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsB // Continue walking types return false; + + static void handleFunctionPointerType(FunctionPointerTypeSymbol funcPtr, ArrayBuilder transformFlagsBuilder, bool isNestedNamedType, bool addCustomModifierFlags) + { + Func, bool), bool, bool> visitor = addFlags; + + transformFlagsBuilder.Add(false); + + var sig = funcPtr.Signature; + handle(sig.RefKind, sig.RefCustomModifiers, sig.ReturnTypeWithAnnotations); + + foreach (var param in sig.Parameters) + { + handle(param.RefKind, param.RefCustomModifiers, param.TypeWithAnnotations); + } + + void handle(RefKind refKind, ImmutableArray customModifiers, TypeWithAnnotations twa) + { + if (refKind != RefKind.None) + { + transformFlagsBuilder.Add(false); + } + + if (addCustomModifierFlags) + { + HandleCustomModifiers(customModifiers.Length, transformFlagsBuilder); + HandleCustomModifiers(twa.CustomModifiers.Length, transformFlagsBuilder); + } + + twa.Type.VisitType(visitor, (transformFlagsBuilder, addCustomModifierFlags)); + } + + static bool addFlags(TypeSymbol type, (ArrayBuilder builder, bool addCustomModifiers) param, bool isNestedNamedType) + { + return AddFlags(type, param.builder, isNestedNamedType, param.addCustomModifiers); + } + } } private static void HandleCustomModifiers(int customModifiersCount, ArrayBuilder transformFlagsBuilder) diff --git a/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs index d2b1874573460..d5868feaf77c2 100644 --- a/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs @@ -221,6 +221,7 @@ public static int CustomModifierCount(this Symbol m) case SymbolKind.NamedType: case SymbolKind.PointerType: case SymbolKind.TypeParameter: + case SymbolKind.FunctionPointer: return ((TypeSymbol)m).CustomModifierCount(); case SymbolKind.Event: return ((EventSymbol)m).CustomModifierCount(); diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs index 15a5aaf534c45..41837fa860c66 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs @@ -164,6 +164,9 @@ private TypeSymbol TransformType(TypeSymbol type) case SymbolKind.PointerType: return TransformPointerType((PointerTypeSymbol)type); + case SymbolKind.FunctionPointer: + return TransformFunctionPointerType((FunctionPointerTypeSymbol)type); + case SymbolKind.DynamicType: Debug.Assert(!_haveCustomModifierFlags, "This shouldn't happen during decoding."); return ConsumeFlag() @@ -338,6 +341,78 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) new PointerTypeSymbol(pointerType.PointedAtTypeWithAnnotations.WithTypeAndModifiers(transformedPointedAtType, pointerType.PointedAtTypeWithAnnotations.CustomModifiers)); } +#nullable enable + private FunctionPointerTypeSymbol? TransformFunctionPointerType(FunctionPointerTypeSymbol type) + { + var flag = ConsumeFlag(); + Debug.Assert(!flag); + + var sig = type.Signature; + + var transformedReturn = handle(ref this, sig.RefKind, sig.RefCustomModifiers, sig.ReturnTypeWithAnnotations); + if (transformedReturn is null) + { + return null; + } + + var transformedParameters = ImmutableArray.Empty; + var paramsTransformed = false; + if (sig.ParameterCount > 0) + { + var paramsBuilder = ArrayBuilder.GetInstance(sig.ParameterCount); + try + { + foreach (var param in sig.Parameters) + { + var transformedParamType = handle(ref this, param.RefKind, param.RefCustomModifiers, param.TypeWithAnnotations); + if (transformedParamType is null) + { + return null; + } + + if (param.Type.Equals(transformedParamType, TypeCompareKind.ConsiderEverything)) + { + paramsBuilder.Add(param.TypeWithAnnotations); + } + else + { + paramsBuilder.Add(param.TypeWithAnnotations.WithType(transformedParamType)); + paramsTransformed = true; + } + } + + transformedParameters = paramsTransformed ? paramsBuilder.ToImmutable() : sig.ParameterTypesWithAnnotations; + } + finally + { + paramsBuilder.Free(); + } + } + + if (paramsTransformed || !sig.ReturnType.Equals(transformedReturn, TypeCompareKind.ConsiderEverything)) + { + return type.SubstituteTypeSymbol(sig.ReturnTypeWithAnnotations.WithType(transformedReturn), transformedParameters, + refCustomModifiers: default, paramRefCustomModifiers: default); + } + else + { + return type; + } + + static TypeSymbol? handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray customModifiers, TypeWithAnnotations twa) + { + if ((refKind != RefKind.None && decoder.ConsumeFlag()) + || !decoder.HandleCustomModifiers(customModifiers.Length) + || !decoder.HandleCustomModifiers(twa.CustomModifiers.Length)) + { + return null; + } + + return decoder.TransformType(twa.Type); + } + } +#nullable restore + private bool HasFlag => _index < _dynamicTransformFlags.Length || !_checkLength; private bool PeekFlag() => _index < _dynamicTransformFlags.Length && _dynamicTransformFlags[_index]; diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs b/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs index 5507899aa8d35..e6000b9215d90 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs @@ -543,6 +543,9 @@ public bool Is(TypeParameterSymbol other) public TypeWithAnnotations WithTypeAndModifiers(TypeSymbol typeSymbol, ImmutableArray customModifiers) => _extensions.WithTypeAndModifiers(this, typeSymbol, customModifiers); + public TypeWithAnnotations WithType(TypeSymbol typeSymbol) => + _extensions.WithTypeAndModifiers(this, typeSymbol, CustomModifiers); + /// /// Used by callers before calling CSharpCompilation.EnsureNullableAttributeExists(). /// diff --git a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs index 5cb00356f36d2..d569f4188a3f7 100644 --- a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs @@ -43,6 +43,17 @@ public static int CustomModifierCount(this TypeSymbol type) TypeWithAnnotations pointedAtType = pointer.PointedAtTypeWithAnnotations; return pointedAtType.CustomModifiers.Length + pointedAtType.Type.CustomModifierCount(); } + case SymbolKind.FunctionPointer: + { + var funcPtr = (FunctionPointerTypeSymbol)type; + var count = funcPtr.Signature.ReturnTypeWithAnnotations.CustomModifiers.Length + funcPtr.Signature.ReturnType.CustomModifierCount(); + foreach (var param in funcPtr.Signature.Parameters) + { + count += param.TypeWithAnnotations.CustomModifiers.Length + param.Type.CustomModifierCount(); + } + + return count; + } case SymbolKind.ErrorType: case SymbolKind.NamedType: { @@ -96,14 +107,31 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA { var array = (ArrayTypeSymbol)type; TypeWithAnnotations elementType = array.ElementTypeWithAnnotations; - return elementType.CustomModifiers.Any() || elementType.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds) || - (flagNonDefaultArraySizesOrLowerBounds && !array.HasDefaultSizesAndLowerBounds); + return checkTypeWithAnnotations(elementType) || (flagNonDefaultArraySizesOrLowerBounds && !array.HasDefaultSizesAndLowerBounds); } case SymbolKind.PointerType: { var pointer = (PointerTypeSymbol)type; TypeWithAnnotations pointedAtType = pointer.PointedAtTypeWithAnnotations; - return pointedAtType.CustomModifiers.Any() || pointedAtType.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds); + return checkTypeWithAnnotations(pointedAtType); + } + case SymbolKind.FunctionPointer: + { + var funcPtr = (FunctionPointerTypeSymbol)type; + if (checkTypeWithAnnotations(funcPtr.Signature.ReturnTypeWithAnnotations)) + { + return true; + } + + foreach (var param in funcPtr.Signature.Parameters) + { + if (checkTypeWithAnnotations(param.TypeWithAnnotations)) + { + return true; + } + } + + return false; } case SymbolKind.ErrorType: case SymbolKind.NamedType: @@ -119,7 +147,7 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA foreach (TypeWithAnnotations typeArg in typeArgs) { - if (!typeArg.CustomModifiers.IsEmpty || typeArg.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds)) + if (checkTypeWithAnnotations(typeArg)) { return true; } @@ -133,6 +161,8 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA } return false; + + bool checkTypeWithAnnotations(TypeWithAnnotations twa) => twa.CustomModifiers.Any() || twa.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds); } /// diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index c6f8ca2d9c492..08abc9c8f5d09 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -5175,6 +5175,225 @@ void symbolValidator(ModuleSymbol module) } } + [Fact] + public void DynamicTypeAttributeInMetadata() + { + var comp = CompileAndVerifyFunctionPointers(@" +#pragma warning disable CS0649 // Unassigned field +unsafe class C +{ + public delegate* F1; + + public delegate* F2; + public delegate* F3; + public delegate* F4; + + public delegate* F5; + + public delegate* F6; + public delegate* F7; + public delegate* F8; + + public delegate* F9; + public delegate* F10; + public delegate* F11; + + public delegate* F12; + public delegate* F13; + + public delegate* F14; +} +", symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol module) + { + var c = module.GlobalNamespace.GetTypeMember("C"); + + assertField("F1", "System.Runtime.CompilerServices.DynamicAttribute({false, true, true, true})", "delegate*"); + + assertField("F2", "System.Runtime.CompilerServices.DynamicAttribute({false, true, false, false})", "delegate*"); + assertField("F3", "System.Runtime.CompilerServices.DynamicAttribute({false, false, true, false})", "delegate*"); + assertField("F4", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true})", "delegate*"); + + assertField("F5", null, "delegate*"); + + assertField("F6", "System.Runtime.CompilerServices.DynamicAttribute({false, false, true, false, false})", "delegate*"); + assertField("F7", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true, false})", "delegate*"); + assertField("F8", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, true})", "delegate*"); + + assertField("F9", "System.Runtime.CompilerServices.DynamicAttribute({false, true, false, false, false, false})", "delegate*"); + assertField("F10", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true, false, false})", "delegate*"); + assertField("F11", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, false, true})", "delegate*"); + + assertField("F12", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true, false})", "delegate*"); + assertField("F13", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, true})", "delegate*"); + + assertField("F14", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, true})", "delegate*"); + + void assertField(string field, string? expectedAttribute, string expectedType) + { + var f = c.GetField(field); + if (expectedAttribute is null) + { + Assert.Empty(f.GetAttributes()); + } + else + { + Assert.Equal(expectedAttribute, f.GetAttributes().Single().ToString()); + } + + CommonVerifyFunctionPointer((FunctionPointerTypeSymbol)f.Type); + Assert.Equal(expectedType, f.Type.ToTestDisplayString()); + } + } + } + + [Fact] + public void DynamicOverriddenWithCustomModifiers() + { + var il = @" +.class public A +{ + .method public hidebysig newslot virtual + instance void M(method class [mscorlib]System.Object modopt([mscorlib]System.Object) & modopt([mscorlib]System.Object) modreq([mscorlib]System.Runtime.InteropServices.InAttribute) *(class [mscorlib]System.Object modopt([mscorlib]System.Object)) a) managed + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 08 00 00 00 00 00 00 00 00 01 00 01 00 00 ) + ret + } + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + .maxstack 8 + + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} +"; + + var source = @" +unsafe class B : A +{ + public override void M(delegate* a) {} +}"; + + var verifier = CompileAndVerifyFunctionPointersWithIl(source, il, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol module) + { + var b = module.GlobalNamespace.GetTypeMember("B"); + + var m = b.GetMethod("M"); + var param = m.Parameters.Single(); + Assert.Equal("System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, false, true, false, true})", param.GetAttributes().Single().ToString()); + + CommonVerifyFunctionPointer((FunctionPointerTypeSymbol)param.Type); + Assert.Equal("delegate*", param.Type.ToTestDisplayString()); + } + } + + [Fact] + public void BadDynamicAttributes() + { + var il = @" +.class public A +{ + .method public hidebysig static void TooManyFlags(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void TooFewFlags_MissingParam(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 02 00 00 00 00 01 00 00 ) + ret + } + .method public hidebysig static void TooFewFlags_MissingReturn(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 01 00 00 00 00 00 00 ) + ret + } + .method public hidebysig static void PtrTypeIsTrue(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 03 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void NonObjectIsTrue(method class [mscorlib]System.String *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 03 00 00 00 00 01 01 00 00 ) + ret + } + .method public hidebysig static void RefIsTrue_Return(method class [mscorlib]System.Object& *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void RefIsTrue_Param(method class [mscorlib]System.Object *(class [mscorlib]System.Object&) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void ModIsTrue_Return(method class [mscorlib]System.Object modopt([mscorlib]System.Object) *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void ModIsTrue_Param(method class [mscorlib]System.Object *(class [mscorlib]System.Object modopt([mscorlib]System.Object)) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void ModIsTrue_RefReturn(method class [mscorlib]System.Object & modopt([mscorlib]System.Object) *(class [mscorlib]System.Object) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 05 00 00 00 00 00 01 01 01 00 00 ) + ret + } + .method public hidebysig static void ModIsTrue_RefParam(method class [mscorlib]System.Object *(class [mscorlib]System.Object & modopt([mscorlib]System.Object)) a) + { + .param [1] + .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 05 00 00 00 00 01 00 01 01 00 00 ) + ret + } +} +"; + + var comp = CreateCompilationWithFunctionPointersAndIl("", il); + + var a = comp.GetTypeByMetadataName("A"); + + assert("TooManyFlags", "delegate*"); + assert("TooFewFlags_MissingParam", "delegate*"); + assert("TooFewFlags_MissingReturn", "delegate*"); + assert("PtrTypeIsTrue", "delegate*"); + assert("NonObjectIsTrue", "delegate*"); + assert("RefIsTrue_Return", "delegate*"); + assert("RefIsTrue_Param", "delegate*"); + assert("ModIsTrue_Return", "delegate*"); + assert("ModIsTrue_Param", "delegate*"); + assert("ModIsTrue_RefReturn", "delegate*"); + assert("ModIsTrue_RefParam", "delegate*"); + + void assert(string methodName, string expectedType) + { + var method = a.GetMethod(methodName); + var param = method.Parameters.Single(); + CommonVerifyFunctionPointer((FunctionPointerTypeSymbol)param.Type); + Assert.Equal(expectedType, param.Type.ToTestDisplayString()); + } + } + private static readonly Guid s_guid = new Guid("97F4DBD4-F6D1-4FAD-91B3-1001F92068E5"); private static readonly BlobContentId s_contentId = new BlobContentId(s_guid, 0x04030201); From c7e9953235c9f6bd2b29719c5dc80701efc92ace Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 6 May 2020 15:51:36 -0700 Subject: [PATCH 03/11] Add support for function pointer native integer decoding. --- .../Symbols/Compilation_WellKnownMembers.cs | 1 + .../Metadata/PE/NativeIntegerTypeDecoder.cs | 41 +++++++++++++++++ .../AttributeTests_NativeInteger.cs | 46 +++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index 06648fb91768e..e6f2f3b396838 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -978,6 +978,7 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder builder, bool i { case TypeKind.Array: case TypeKind.Pointer: + case TypeKind.FunctionPointer: case TypeKind.TypeParameter: case TypeKind.Dynamic: builder.Add(false); diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NativeIntegerTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NativeIntegerTypeDecoder.cs index 26765108afd72..98e85b1f567cd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NativeIntegerTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NativeIntegerTypeDecoder.cs @@ -75,6 +75,8 @@ private TypeSymbol TransformType(TypeSymbol type) return TransformArrayType((ArrayTypeSymbol)type); case TypeKind.Pointer: return TransformPointerType((PointerTypeSymbol)type); + case TypeKind.FunctionPointer: + return TransformFunctionPointerType((FunctionPointerTypeSymbol)type); case TypeKind.TypeParameter: case TypeKind.Dynamic: IgnoreIndex(); @@ -137,6 +139,45 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol type) return type.WithPointedAtType(TransformTypeWithAnnotations(type.PointedAtTypeWithAnnotations)); } + private FunctionPointerTypeSymbol TransformFunctionPointerType(FunctionPointerTypeSymbol type) + { + IgnoreIndex(); + + var transformedReturnType = TransformTypeWithAnnotations(type.Signature.ReturnTypeWithAnnotations); + var transformedParameterTypes = ImmutableArray.Empty; + var paramsModified = false; + + if (type.Signature.ParameterCount > 0) + { + var builder = ArrayBuilder.GetInstance(type.Signature.ParameterCount); + foreach (var param in type.Signature.Parameters) + { + var transformedParam = TransformTypeWithAnnotations(param.TypeWithAnnotations); + paramsModified = paramsModified || !transformedParam.IsSameAs(param.TypeWithAnnotations); + builder.Add(transformedParam); + } + + if (paramsModified) + { + transformedParameterTypes = builder.ToImmutableAndFree(); + } + else + { + transformedParameterTypes = type.Signature.ParameterTypesWithAnnotations; + builder.Free(); + } + } + + if (paramsModified || !transformedReturnType.IsSameAs(type.Signature.ReturnTypeWithAnnotations)) + { + return type.SubstituteTypeSymbol(transformedReturnType, transformedParameterTypes, refCustomModifiers: default, paramRefCustomModifiers: default); + } + else + { + return type; + } + } + private int Increment() { if (_index < _transformFlags.Length) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs index a1dd37f7a9377..38096d8570d4e 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs @@ -1022,6 +1022,52 @@ void assert(string expectedType, string fieldName) } } + [Fact] + public void FunctionPointersWithNativeIntegerTypes() + { + var comp = CompileAndVerify(@" +unsafe class C +{ + public delegate* F1; + public delegate* F2; + public delegate* F3; + public delegate* F4; + public delegate*, nint> F5; + public delegate*> F6; +} +", options: TestOptions.UnsafeReleaseDll, parseOptions: TestOptions.RegularPreview, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol module) + { + var expectedAttributes = @" +C + [NativeInteger({ False, True, True, True })] delegate* F1 + [NativeInteger({ False, True, False, False })] delegate* F2 + [NativeInteger({ False, False, True, False })] delegate* F3 + [NativeInteger({ False, False, False, True })] delegate* F4 + [NativeInteger({ False, True, False, False, False, False })] delegate*, System.IntPtr> F5 + [NativeInteger({ False, False, False, False, False, True })] delegate*> F6 +"; + + AssertNativeIntegerAttributes(module, expectedAttributes); + var c = module.GlobalNamespace.GetTypeMember("C"); + + assert("delegate*", "F1"); + assert("delegate*", "F2"); + assert("delegate*", "F3"); + assert("delegate*", "F4"); + assert("delegate*, nint>", "F5"); + assert("delegate*>", "F6"); + + void assert(string expectedType, string fieldName) + { + var field = c.GetField(fieldName); + FunctionPointerUtilities.CommonVerifyFunctionPointer((FunctionPointerTypeSymbol)field.Type); + Assert.Equal(expectedType, c.GetField(fieldName).Type.ToTestDisplayString()); + } + } + } + private static TypeDefinition GetTypeDefinitionByName(MetadataReader reader, string name) { return reader.GetTypeDefinition(reader.TypeDefinitions.Single(h => reader.StringComparer.Equals(reader.GetTypeDefinition(h).Name, name))); From 5c72cd16aa895c92a189f5ae00adc6729bd96c29 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 8 May 2020 09:32:31 -0700 Subject: [PATCH 04/11] Address feedback from code review. --- .../Portable/Symbols/TypeSymbolExtensions.cs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs index b80b0750a465d..94cade87fbeef 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs @@ -696,9 +696,15 @@ private static bool IsTypeLessVisibleThan(TypeSymbol type, Symbol sym, ref HashS static TypeSymbol? visitFunctionPointerType(FunctionPointerTypeSymbol type, Func? typeWithAnnotationsPredicate, Func? typePredicate, T arg, bool useDefaultType, bool canDigThroughNullable) { - MethodSymbol currentPointer = type.Signature; - var result = visitType(currentPointer.ReturnTypeWithAnnotations); + var result = VisitType( + typeWithAnnotationsOpt: canDigThroughNullable ? default : currentPointer.ReturnTypeWithAnnotations, + type: canDigThroughNullable ? currentPointer.ReturnTypeWithAnnotations.NullableUnderlyingTypeOrSelf : null, + typeWithAnnotationsPredicate, + typePredicate, + arg, + canDigThroughNullable, + useDefaultType); if (result is object) { return result; @@ -706,7 +712,14 @@ private static bool IsTypeLessVisibleThan(TypeSymbol type, Symbol sym, ref HashS foreach (var parameter in currentPointer.Parameters) { - result = visitType(parameter.TypeWithAnnotations); + result = VisitType( + typeWithAnnotationsOpt: canDigThroughNullable ? default : parameter.TypeWithAnnotations, + type: canDigThroughNullable ? parameter.TypeWithAnnotations.NullableUnderlyingTypeOrSelf : null, + typeWithAnnotationsPredicate, + typePredicate, + arg, + canDigThroughNullable, + useDefaultType); if (result is object) { return result; @@ -714,15 +727,6 @@ private static bool IsTypeLessVisibleThan(TypeSymbol type, Symbol sym, ref HashS } return null; - - TypeSymbol? visitType(TypeWithAnnotations typeArg) => VisitType( - typeWithAnnotationsOpt: canDigThroughNullable ? default : typeArg, - type: canDigThroughNullable ? typeArg.NullableUnderlyingTypeOrSelf : null, - typeWithAnnotationsPredicate, - typePredicate, - arg, - canDigThroughNullable, - useDefaultType); } } From a9cfbe23c517b79ec98a7c123a1e6a5dc25f85b9 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 7 May 2020 16:32:06 -0700 Subject: [PATCH 05/11] Add support for breaking ties via modopts on function pointer types. --- .../Utilities/TypeSymbolExtensions.cs | 39 ++--- .../CodeGen/CodeGenFunctionPointersTests.cs | 151 ++++++++++++++++++ 2 files changed, 166 insertions(+), 24 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs index d569f4188a3f7..f73c2d1329290 100644 --- a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs @@ -2,15 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Linq; -using Microsoft.CodeAnalysis.Collections; -using Microsoft.CodeAnalysis.CSharp.Symbols; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -34,25 +29,16 @@ public static int CustomModifierCount(this TypeSymbol type) case SymbolKind.ArrayType: { var array = (ArrayTypeSymbol)type; - TypeWithAnnotations elementType = array.ElementTypeWithAnnotations; - return elementType.CustomModifiers.Length + elementType.Type.CustomModifierCount(); + return customModifierCountForTypeWithAnnotations(array.ElementTypeWithAnnotations); } case SymbolKind.PointerType: { var pointer = (PointerTypeSymbol)type; - TypeWithAnnotations pointedAtType = pointer.PointedAtTypeWithAnnotations; - return pointedAtType.CustomModifiers.Length + pointedAtType.Type.CustomModifierCount(); + return customModifierCountForTypeWithAnnotations(pointer.PointedAtTypeWithAnnotations); } case SymbolKind.FunctionPointer: { - var funcPtr = (FunctionPointerTypeSymbol)type; - var count = funcPtr.Signature.ReturnTypeWithAnnotations.CustomModifiers.Length + funcPtr.Signature.ReturnType.CustomModifierCount(); - foreach (var param in funcPtr.Signature.Parameters) - { - count += param.TypeWithAnnotations.CustomModifiers.Length + param.Type.CustomModifierCount(); - } - - return count; + return ((FunctionPointerTypeSymbol)type).Signature.CustomModifierCount(); } case SymbolKind.ErrorType: case SymbolKind.NamedType: @@ -70,7 +56,7 @@ public static int CustomModifierCount(this TypeSymbol type) foreach (TypeWithAnnotations typeArg in typeArgs) { - count += typeArg.Type.CustomModifierCount() + typeArg.CustomModifiers.Length; + count += customModifierCountForTypeWithAnnotations(typeArg); } namedType = namedType.ContainingType; @@ -83,6 +69,9 @@ public static int CustomModifierCount(this TypeSymbol type) } return 0; + + static int customModifierCountForTypeWithAnnotations(TypeWithAnnotations typeWithAnnotations) + => typeWithAnnotations.CustomModifiers.Length + typeWithAnnotations.Type.CustomModifierCount(); } /// @@ -107,25 +96,26 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA { var array = (ArrayTypeSymbol)type; TypeWithAnnotations elementType = array.ElementTypeWithAnnotations; - return checkTypeWithAnnotations(elementType) || (flagNonDefaultArraySizesOrLowerBounds && !array.HasDefaultSizesAndLowerBounds); + return checkTypeWithAnnotations(elementType, flagNonDefaultArraySizesOrLowerBounds) + || (flagNonDefaultArraySizesOrLowerBounds && !array.HasDefaultSizesAndLowerBounds); } case SymbolKind.PointerType: { var pointer = (PointerTypeSymbol)type; TypeWithAnnotations pointedAtType = pointer.PointedAtTypeWithAnnotations; - return checkTypeWithAnnotations(pointedAtType); + return checkTypeWithAnnotations(pointedAtType, flagNonDefaultArraySizesOrLowerBounds); } case SymbolKind.FunctionPointer: { var funcPtr = (FunctionPointerTypeSymbol)type; - if (checkTypeWithAnnotations(funcPtr.Signature.ReturnTypeWithAnnotations)) + if (checkTypeWithAnnotations(funcPtr.Signature.ReturnTypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) { return true; } foreach (var param in funcPtr.Signature.Parameters) { - if (checkTypeWithAnnotations(param.TypeWithAnnotations)) + if (checkTypeWithAnnotations(param.TypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) { return true; } @@ -147,7 +137,7 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA foreach (TypeWithAnnotations typeArg in typeArgs) { - if (checkTypeWithAnnotations(typeArg)) + if (checkTypeWithAnnotations(typeArg, flagNonDefaultArraySizesOrLowerBounds)) { return true; } @@ -162,7 +152,8 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA return false; - bool checkTypeWithAnnotations(TypeWithAnnotations twa) => twa.CustomModifiers.Any() || twa.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds); + static bool checkTypeWithAnnotations(TypeWithAnnotations typeWithAnnotations, bool flagNonDefaultArraySizesOrLowerBounds) + => typeWithAnnotations.CustomModifiers.Any() || typeWithAnnotations.Type.HasCustomModifiers(flagNonDefaultArraySizesOrLowerBounds); } /// diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index 08abc9c8f5d09..b91bffb4f2700 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -5394,6 +5394,157 @@ void assert(string methodName, string expectedType) } } + [Fact] + public void BetterFunctionMember_BreakTiesByCustomModifierCount_TypeMods() + { + var il = @" +.class public auto ansi beforefieldinit Program + extends [mscorlib]System.Object +{ + // Methods + .method public hidebysig static + void RetModifiers (method void modopt([mscorlib]System.Object) modopt([mscorlib]System.Object) *()) cil managed + { + ldstr ""M"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void RetModifiers (method void modopt([mscorlib]System.Object) *()) cil managed + { + ldstr ""L"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void ParamModifiers (method void *(int32 modopt([mscorlib]System.Object) modopt([mscorlib]System.Object))) cil managed + { + ldstr ""M"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void ParamModifiers (method void *(int32) modopt([mscorlib]System.Object)) cil managed + { + ldstr ""L"" + call void [mscorlib]System.Console::Write(string) + ret + } +}"; + + var source = @" +unsafe class C +{ + static void Main() + { + delegate* ptr1 = null; + Program.RetModifiers(ptr1); + delegate* ptr2 = null; + Program.ParamModifiers(ptr2); + } +}"; + + var verifier = CompileAndVerifyFunctionPointersWithIl(source, il, expectedOutput: "LL"); + verifier.VerifyIL("C.Main", expectedIL: @" +{ + // Code size 19 (0x13) + .maxstack 1 + .locals init (delegate* V_0, //ptr1 + delegate* V_1) //ptr2 + IL_0000: ldc.i4.0 + IL_0001: conv.u + IL_0002: stloc.0 + IL_0003: ldloc.0 + IL_0004: call ""void Program.RetModifiers(delegate*)"" + IL_0009: ldc.i4.0 + IL_000a: conv.u + IL_000b: stloc.1 + IL_000c: ldloc.1 + IL_000d: call ""void Program.ParamModifiers(delegate*)"" + IL_0012: ret +} + "); + } + + [Fact] + public void BetterFunctionMember_BreakTiesByCustomModifierCount_Ref() + { + var il = @" +.class public auto ansi beforefieldinit Program + extends [mscorlib]System.Object +{ + // Methods + .method public hidebysig static + void RetModifiers (method int32 & modopt([mscorlib]System.Object) modopt([mscorlib]System.Object) *()) cil managed + { + ldstr ""M"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void RetModifiers (method int32 & modopt([mscorlib]System.Object) *()) cil managed + { + ldstr ""L"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void ParamModifiers (method void *(int32 & modopt([mscorlib]System.Object) modopt([mscorlib]System.Object))) cil managed + { + ldstr ""M"" + call void [mscorlib]System.Console::Write(string) + ret + } + + .method public hidebysig static + void ParamModifiers (method void *(int32 & modopt([mscorlib]System.Object))) cil managed + { + ldstr ""L"" + call void [mscorlib]System.Console::Write(string) + ret + } +} +"; + + var source = @" +unsafe class C +{ + static void Main() + { + delegate* ptr1 = null; + Program.RetModifiers(ptr1); + delegate* ptr2 = null; + Program.ParamModifiers(ptr2); + } +}"; + + var verifier = CompileAndVerifyFunctionPointersWithIl(source, il, expectedOutput: "LL"); + verifier.VerifyIL("C.Main", expectedIL: @" +{ + // Code size 19 (0x13) + .maxstack 1 + .locals init (delegate* V_0, //ptr1 + delegate* V_1) //ptr2 + IL_0000: ldc.i4.0 + IL_0001: conv.u + IL_0002: stloc.0 + IL_0003: ldloc.0 + IL_0004: call ""void Program.RetModifiers(delegate*)"" + IL_0009: ldc.i4.0 + IL_000a: conv.u + IL_000b: stloc.1 + IL_000c: ldloc.1 + IL_000d: call ""void Program.ParamModifiers(delegate*)"" + IL_0012: ret +} +"); + } + private static readonly Guid s_guid = new Guid("97F4DBD4-F6D1-4FAD-91B3-1001F92068E5"); private static readonly BlobContentId s_contentId = new BlobContentId(s_guid, 0x04030201); From 15c3201b7fac78c5eac4701e4e3aa9345cf7adbb Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 11 May 2020 16:47:04 -0700 Subject: [PATCH 06/11] PR Feedback. --- .../SymbolDisplayVisitor.Members.cs | 5 --- .../Symbols/Compilation_WellKnownMembers.cs | 24 +++++++------ .../Symbols/Metadata/PE/DynamicTypeDecoder.cs | 34 +++++++++++++------ .../AttributeTests_NativeInteger.cs | 8 ++++- .../CodeGen/CodeGenFunctionPointersTests.cs | 28 ++++++++++++--- 5 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs index a4e171fd055b9..d985d4cbcf285 100644 --- a/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs +++ b/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs @@ -280,11 +280,6 @@ public override void VisitMethod(IMethodSymbol symbol) AddSpace(); } - if (ShouldMethodDisplayReadOnly(symbol)) - { - AddReadOnlyIfRequired(); - } - if (symbol.ReturnsByRef) { AddRefIfRequired(); diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index e6f2f3b396838..17a3675c218ef 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -862,7 +862,7 @@ internal static void Encode(TypeSymbol type, int customModifiersCount, RefKind r private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsBuilder, bool isNestedNamedType, bool addCustomModifierFlags) { - // Encode transforms flag for this type and it's custom modifiers (if any). + // Encode transforms flag for this type and its custom modifiers (if any). switch (type.TypeKind) { case TypeKind.Dynamic: @@ -888,10 +888,13 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsB break; case TypeKind.FunctionPointer: - handleFunctionPointerType((FunctionPointerTypeSymbol)type, transformFlagsBuilder, isNestedNamedType, addCustomModifierFlags); + Debug.Assert(!isNestedNamedType); + handleFunctionPointerType((FunctionPointerTypeSymbol)type, transformFlagsBuilder, addCustomModifierFlags); // Function pointer types have nested custom modifiers and refkinds in line with types, and visit all their nested types // as part of this call. + // We need a different way to indicate that we should not recurse for this type, but should continue walking for other + // types. https://github.com/dotnet/roslyn/issues/44160 return true; default: @@ -914,10 +917,12 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsB // Continue walking types return false; - static void handleFunctionPointerType(FunctionPointerTypeSymbol funcPtr, ArrayBuilder transformFlagsBuilder, bool isNestedNamedType, bool addCustomModifierFlags) + static void handleFunctionPointerType(FunctionPointerTypeSymbol funcPtr, ArrayBuilder transformFlagsBuilder, bool addCustomModifierFlags) { - Func, bool), bool, bool> visitor = addFlags; + Func, bool), bool, bool> visitor = + (TypeSymbol type, (ArrayBuilder builder, bool addCustomModiferFlags) param, bool _) => AddFlags(type, param.builder, isNestedNamedType: false, param.addCustomModiferFlags); + // The function pointer type itself gets a false transformFlagsBuilder.Add(false); var sig = funcPtr.Signature; @@ -930,6 +935,11 @@ static void handleFunctionPointerType(FunctionPointerTypeSymbol funcPtr, ArrayBu void handle(RefKind refKind, ImmutableArray customModifiers, TypeWithAnnotations twa) { + if (addCustomModifierFlags) + { + HandleCustomModifiers(customModifiers.Length, transformFlagsBuilder); + } + if (refKind != RefKind.None) { transformFlagsBuilder.Add(false); @@ -937,17 +947,11 @@ void handle(RefKind refKind, ImmutableArray customModifiers, Typ if (addCustomModifierFlags) { - HandleCustomModifiers(customModifiers.Length, transformFlagsBuilder); HandleCustomModifiers(twa.CustomModifiers.Length, transformFlagsBuilder); } twa.Type.VisitType(visitor, (transformFlagsBuilder, addCustomModifierFlags)); } - - static bool addFlags(TypeSymbol type, (ArrayBuilder builder, bool addCustomModifiers) param, bool isNestedNamedType) - { - return AddFlags(type, param.builder, isNestedNamedType, param.addCustomModifiers); - } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs index 41837fa860c66..10cadd0a50dc7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs @@ -119,7 +119,7 @@ private static TypeSymbol TransformTypeInternal( var decoder = new DynamicTypeDecoder(dynamicTransformFlags, haveCustomModifierFlags, checkLength, containingAssembly); // Native compiler encodes bools (always false) for custom modifiers and parameter ref-kinds, if ref-kind is ref or out. - if (decoder.HandleCustomModifiers(targetSymbolCustomModifierCount) && decoder.HandleParameterRefKind(targetSymbolRefKind)) + if (decoder.HandleCustomModifiers(targetSymbolCustomModifierCount) && decoder.HandleRefKind(targetSymbolRefKind)) { TypeSymbol transformedType = decoder.TransformType(metadataType); @@ -204,7 +204,7 @@ private bool HandleCustomModifiers(int customModifiersCount) } // Native compiler encodes bools (always false) for custom modifiers and parameter ref-kinds, if ref-kind is ref or out. - private bool HandleParameterRefKind(RefKind refKind) + private bool HandleRefKind(RefKind refKind) { Debug.Assert(_index >= 0); return refKind == RefKind.None || !ConsumeFlag(); @@ -355,10 +355,23 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) return null; } + bool madeChanges = false; + TypeWithAnnotations transformedReturnWithAnnotations; + + if (!transformedReturn.Equals(sig.ReturnType, TypeCompareKind.ConsiderEverything)) + { + transformedReturnWithAnnotations = sig.ReturnTypeWithAnnotations.WithType(transformedReturn); + madeChanges = true; + } + else + { + transformedReturnWithAnnotations = sig.ReturnTypeWithAnnotations; + } + var transformedParameters = ImmutableArray.Empty; - var paramsTransformed = false; if (sig.ParameterCount > 0) { + var paramsTransformed = false; var paramsBuilder = ArrayBuilder.GetInstance(sig.ParameterCount); try { @@ -382,6 +395,7 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) } transformedParameters = paramsTransformed ? paramsBuilder.ToImmutable() : sig.ParameterTypesWithAnnotations; + madeChanges |= paramsTransformed; } finally { @@ -389,9 +403,9 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) } } - if (paramsTransformed || !sig.ReturnType.Equals(transformedReturn, TypeCompareKind.ConsiderEverything)) + if (madeChanges) { - return type.SubstituteTypeSymbol(sig.ReturnTypeWithAnnotations.WithType(transformedReturn), transformedParameters, + return type.SubstituteTypeSymbol(transformedReturnWithAnnotations, transformedParameters, refCustomModifiers: default, paramRefCustomModifiers: default); } else @@ -399,16 +413,16 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) return type; } - static TypeSymbol? handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray customModifiers, TypeWithAnnotations twa) + static TypeSymbol? handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray refCustomModifiers, TypeWithAnnotations typeWithAnnotations) { - if ((refKind != RefKind.None && decoder.ConsumeFlag()) - || !decoder.HandleCustomModifiers(customModifiers.Length) - || !decoder.HandleCustomModifiers(twa.CustomModifiers.Length)) + if (!decoder.HandleCustomModifiers(refCustomModifiers.Length) + || !decoder.HandleRefKind(refKind) + || !decoder.HandleCustomModifiers(typeWithAnnotations.CustomModifiers.Length)) { return null; } - return decoder.TransformType(twa.Type); + return decoder.TransformType(typeWithAnnotations.Type); } } #nullable restore diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs index 38096d8570d4e..fb11b2dc124d6 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_NativeInteger.cs @@ -1034,10 +1034,12 @@ unsafe class C public delegate* F4; public delegate*, nint> F5; public delegate*> F6; + public delegate*, System.IntPtr> F7; + public delegate*> F8; } ", options: TestOptions.UnsafeReleaseDll, parseOptions: TestOptions.RegularPreview, symbolValidator: symbolValidator); - void symbolValidator(ModuleSymbol module) + static void symbolValidator(ModuleSymbol module) { var expectedAttributes = @" C @@ -1047,6 +1049,8 @@ void symbolValidator(ModuleSymbol module) [NativeInteger({ False, False, False, True })] delegate* F4 [NativeInteger({ False, True, False, False, False, False })] delegate*, System.IntPtr> F5 [NativeInteger({ False, False, False, False, False, True })] delegate*> F6 + [NativeInteger({ False, False, False, True, False, False })] delegate*, System.IntPtr> F7 + [NativeInteger({ False, False, False, False, True, False })] delegate*> F8 "; AssertNativeIntegerAttributes(module, expectedAttributes); @@ -1058,6 +1062,8 @@ void symbolValidator(ModuleSymbol module) assert("delegate*", "F4"); assert("delegate*, nint>", "F5"); assert("delegate*>", "F6"); + assert("delegate*, System.IntPtr>", "F7"); + assert("delegate*>", "F8"); void assert(string expectedType, string fieldName) { diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index b91bffb4f2700..5cc369fdbf0c9 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -5202,7 +5202,10 @@ unsafe class C public delegate* F13; public delegate* F14; + + public D[], dynamic> F15; } +class D { } ", symbolValidator: symbolValidator); void symbolValidator(ModuleSymbol module) @@ -5230,6 +5233,9 @@ void symbolValidator(ModuleSymbol module) assertField("F14", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, false, true})", "delegate*"); + // https://github.com/dotnet/roslyn/issues/44160 tracks fixing this. We're not encoding dynamic correctly for function pointers as type parameters + assertField("F15", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true})", "D[], System.Object>"); + void assertField(string field, string? expectedAttribute, string expectedType) { var f = c.GetField(field); @@ -5242,7 +5248,10 @@ void assertField(string field, string? expectedAttribute, string expectedType) Assert.Equal(expectedAttribute, f.GetAttributes().Single().ToString()); } - CommonVerifyFunctionPointer((FunctionPointerTypeSymbol)f.Type); + if (f.Type is FunctionPointerTypeSymbol ptrType) + { + CommonVerifyFunctionPointer(ptrType); + } Assert.Equal(expectedType, f.Type.ToTestDisplayString()); } } @@ -5264,8 +5273,6 @@ .param [1] .method public hidebysig specialname rtspecialname instance void .ctor () cil managed { - .maxstack 8 - ldarg.0 call instance void [mscorlib]System.Object::.ctor() ret @@ -5281,7 +5288,7 @@ public override void M(delegate* a) {} var verifier = CompileAndVerifyFunctionPointersWithIl(source, il, symbolValidator: symbolValidator); - void symbolValidator(ModuleSymbol module) + static void symbolValidator(ModuleSymbol module) { var b = module.GlobalNamespace.GetTypeMember("B"); @@ -5303,66 +5310,77 @@ .class public A .method public hidebysig static void TooManyFlags(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) { .param [1] + //{false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void TooFewFlags_MissingParam(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) { .param [1] + //{false, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 02 00 00 00 00 01 00 00 ) ret } .method public hidebysig static void TooFewFlags_MissingReturn(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) { .param [1] + //{false} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 01 00 00 00 00 00 00 ) ret } .method public hidebysig static void PtrTypeIsTrue(method class [mscorlib]System.Object *(class [mscorlib]System.Object) a) { .param [1] + //{true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 03 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void NonObjectIsTrue(method class [mscorlib]System.String *(class [mscorlib]System.Object) a) { .param [1] + //{false, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 03 00 00 00 00 01 01 00 00 ) ret } .method public hidebysig static void RefIsTrue_Return(method class [mscorlib]System.Object& *(class [mscorlib]System.Object) a) { .param [1] + //{false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void RefIsTrue_Param(method class [mscorlib]System.Object *(class [mscorlib]System.Object&) a) { .param [1] + //{false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void ModIsTrue_Return(method class [mscorlib]System.Object modopt([mscorlib]System.Object) *(class [mscorlib]System.Object) a) { .param [1] + //{false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void ModIsTrue_Param(method class [mscorlib]System.Object *(class [mscorlib]System.Object modopt([mscorlib]System.Object)) a) { .param [1] + //{false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 04 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void ModIsTrue_RefReturn(method class [mscorlib]System.Object & modopt([mscorlib]System.Object) *(class [mscorlib]System.Object) a) { .param [1] + //{false, false, true, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 05 00 00 00 00 00 01 01 01 00 00 ) ret } .method public hidebysig static void ModIsTrue_RefParam(method class [mscorlib]System.Object *(class [mscorlib]System.Object & modopt([mscorlib]System.Object)) a) { .param [1] + //{false, true, false, true, true} .custom instance void [mscorlib]System.Runtime.CompilerServices.DynamicAttribute::.ctor(bool[]) = ( 01 00 05 00 00 00 00 01 00 01 01 00 00 ) ret } @@ -5543,7 +5561,7 @@ .locals init (delegate* V_0, //ptr1 IL_0012: ret } "); - } + } private static readonly Guid s_guid = new Guid("97F4DBD4-F6D1-4FAD-91B3-1001F92068E5"); private static readonly BlobContentId s_contentId = new BlobContentId(s_guid, 0x04030201); From 9d7b5510c96d76b03dc6299860d56ddc68bdc869 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 11 May 2020 17:10:19 -0700 Subject: [PATCH 07/11] Include ref custom modifiers in check. --- .../CSharp/Portable/Utilities/TypeSymbolExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs index f73c2d1329290..a0e1d42448327 100644 --- a/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Utilities/TypeSymbolExtensions.cs @@ -108,14 +108,14 @@ public static bool HasCustomModifiers(this TypeSymbol type, bool flagNonDefaultA case SymbolKind.FunctionPointer: { var funcPtr = (FunctionPointerTypeSymbol)type; - if (checkTypeWithAnnotations(funcPtr.Signature.ReturnTypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) + if (!funcPtr.Signature.RefCustomModifiers.IsEmpty || checkTypeWithAnnotations(funcPtr.Signature.ReturnTypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) { return true; } foreach (var param in funcPtr.Signature.Parameters) { - if (checkTypeWithAnnotations(param.TypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) + if (!param.RefCustomModifiers.IsEmpty || checkTypeWithAnnotations(param.TypeWithAnnotations, flagNonDefaultArraySizesOrLowerBounds)) { return true; } From dfd1bc2b66c0d1d1998868d184f8a4c4bc38892f Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 12 May 2020 10:42:25 -0700 Subject: [PATCH 08/11] Extract common logic in DynamicTypeDecoder, add formatting test. --- .../Symbols/Metadata/PE/DynamicTypeDecoder.cs | 49 ++++++++----------- .../Symbols/FunctionPointerTypeSymbolTests.cs | 25 ++++++++++ 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs index 10cadd0a50dc7..afd11e4432697 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs @@ -349,25 +349,12 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) var sig = type.Signature; - var transformedReturn = handle(ref this, sig.RefKind, sig.RefCustomModifiers, sig.ReturnTypeWithAnnotations); - if (transformedReturn is null) + var (transformedReturnWithAnnotations, madeChanges) = handle(ref this, sig.RefKind, sig.RefCustomModifiers, sig.ReturnTypeWithAnnotations); + if (transformedReturnWithAnnotations is null) { return null; } - bool madeChanges = false; - TypeWithAnnotations transformedReturnWithAnnotations; - - if (!transformedReturn.Equals(sig.ReturnType, TypeCompareKind.ConsiderEverything)) - { - transformedReturnWithAnnotations = sig.ReturnTypeWithAnnotations.WithType(transformedReturn); - madeChanges = true; - } - else - { - transformedReturnWithAnnotations = sig.ReturnTypeWithAnnotations; - } - var transformedParameters = ImmutableArray.Empty; if (sig.ParameterCount > 0) { @@ -377,21 +364,14 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) { foreach (var param in sig.Parameters) { - var transformedParamType = handle(ref this, param.RefKind, param.RefCustomModifiers, param.TypeWithAnnotations); + var (transformedParamType, paramTransformed) = handle(ref this, param.RefKind, param.RefCustomModifiers, param.TypeWithAnnotations); if (transformedParamType is null) { return null; } - if (param.Type.Equals(transformedParamType, TypeCompareKind.ConsiderEverything)) - { - paramsBuilder.Add(param.TypeWithAnnotations); - } - else - { - paramsBuilder.Add(param.TypeWithAnnotations.WithType(transformedParamType)); - paramsTransformed = true; - } + paramsBuilder.Add(transformedParamType.Value); + paramsTransformed |= paramTransformed; } transformedParameters = paramsTransformed ? paramsBuilder.ToImmutable() : sig.ParameterTypesWithAnnotations; @@ -405,7 +385,7 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) if (madeChanges) { - return type.SubstituteTypeSymbol(transformedReturnWithAnnotations, transformedParameters, + return type.SubstituteTypeSymbol(transformedReturnWithAnnotations.Value, transformedParameters, refCustomModifiers: default, paramRefCustomModifiers: default); } else @@ -413,16 +393,27 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) return type; } - static TypeSymbol? handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray refCustomModifiers, TypeWithAnnotations typeWithAnnotations) + static (TypeWithAnnotations?, bool madeChanges) handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray refCustomModifiers, TypeWithAnnotations typeWithAnnotations) { if (!decoder.HandleCustomModifiers(refCustomModifiers.Length) || !decoder.HandleRefKind(refKind) || !decoder.HandleCustomModifiers(typeWithAnnotations.CustomModifiers.Length)) { - return null; + return (null, false); + } + + var transformedType = decoder.TransformType(typeWithAnnotations.Type); + if (transformedType is null) + { + return (null, false); + } + + if (transformedType.Equals(typeWithAnnotations.Type, TypeCompareKind.ConsiderEverything)) + { + return (typeWithAnnotations, false); } - return decoder.TransformType(typeWithAnnotations.Type); + return (typeWithAnnotations.WithType(transformedType), true); } } #nullable restore diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/FunctionPointerTypeSymbolTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/FunctionPointerTypeSymbolTests.cs index 410ced62b6a61..878ea86ec968c 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/FunctionPointerTypeSymbolTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/FunctionPointerTypeSymbolTests.cs @@ -1042,5 +1042,30 @@ IEnumerable Iterator2(delegate*[] i) Diagnostic(ErrorCode.ERR_UnsafeIteratorArgType, "i").WithLocation(10, 50) ); } + + [Fact] + public void FormattingReturnTypeOptions() + { + var il = @" +.class public auto ansi beforefieldinit C + extends [mscorlib]System.Object +{ + .field public method int32& modreq([mscorlib]System.Runtime.InteropServices.InAttribute) *() 'Field1' + .field public method int32 modopt([mscorlib]System.Object) & *() 'Field2' +} +"; + + var comp = CreateCompilation("", references: new[] { CompileIL(il) }); + comp.VerifyDiagnostics(); + + var c = comp.GetTypeByMetadataName("C"); + var f1 = c.GetField("Field1").Type; + var f2 = c.GetField("Field2").Type; + + Assert.Equal("delegate*", f1.ToTestDisplayString()); + Assert.Equal("delegate*", f1.ToDisplayString()); + Assert.Equal("delegate*", f2.ToTestDisplayString()); + Assert.Equal("delegate*", f2.ToDisplayString()); + } } } From 68f49f554475dd2e93189bf9be7c1b121e273d1f Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 13 May 2020 13:11:04 -0700 Subject: [PATCH 09/11] PR Feedback. --- .../CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs | 2 +- .../CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs index 17a3675c218ef..790ffc32ff19b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs @@ -920,7 +920,7 @@ private static bool AddFlags(TypeSymbol type, ArrayBuilder transformFlagsB static void handleFunctionPointerType(FunctionPointerTypeSymbol funcPtr, ArrayBuilder transformFlagsBuilder, bool addCustomModifierFlags) { Func, bool), bool, bool> visitor = - (TypeSymbol type, (ArrayBuilder builder, bool addCustomModiferFlags) param, bool _) => AddFlags(type, param.builder, isNestedNamedType: false, param.addCustomModiferFlags); + (TypeSymbol type, (ArrayBuilder builder, bool addCustomModiferFlags) param, bool isNestedNamedType) => AddFlags(type, param.builder, isNestedNamedType, param.addCustomModiferFlags); // The function pointer type itself gets a false transformFlagsBuilder.Add(false); diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs index afd11e4432697..d7ac200d3bcdf 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs @@ -203,7 +203,7 @@ private bool HandleCustomModifiers(int customModifiersCount) return true; } - // Native compiler encodes bools (always false) for custom modifiers and parameter ref-kinds, if ref-kind is ref or out. + // Native compiler encodes bools (always false) for custom modifiers and parameter ref-kinds, if ref-kind is not none. private bool HandleRefKind(RefKind refKind) { Debug.Assert(_index >= 0); From 202f412b0f8cea0621f023583ada97816919171f Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 13 May 2020 17:42:30 -0700 Subject: [PATCH 10/11] Add additional test covering nested named types. --- .../Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index 5cc369fdbf0c9..d015a7a0ef85e 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -5204,8 +5204,14 @@ unsafe class C public delegate* F14; public D[], dynamic> F15; + + public delegate*.B> F16; } class D { } +class A +{ + public class B {} +} ", symbolValidator: symbolValidator); void symbolValidator(ModuleSymbol module) @@ -5236,6 +5242,8 @@ void symbolValidator(ModuleSymbol module) // https://github.com/dotnet/roslyn/issues/44160 tracks fixing this. We're not encoding dynamic correctly for function pointers as type parameters assertField("F15", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true})", "D[], System.Object>"); + assertField("F16", "System.Runtime.CompilerServices.DynamicAttribute({false, false, false, true})", "delegate*.B>"); + void assertField(string field, string? expectedAttribute, string expectedType) { var f = c.GetField(field); From d809b3aa98d2fe452bfead15add257a07a829055 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 13 May 2020 17:51:07 -0700 Subject: [PATCH 11/11] Simplify dynamic decoder --- .../Symbols/Metadata/PE/DynamicTypeDecoder.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs index d7ac200d3bcdf..a6eeeb1985036 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/DynamicTypeDecoder.cs @@ -350,7 +350,7 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) var sig = type.Signature; var (transformedReturnWithAnnotations, madeChanges) = handle(ref this, sig.RefKind, sig.RefCustomModifiers, sig.ReturnTypeWithAnnotations); - if (transformedReturnWithAnnotations is null) + if (transformedReturnWithAnnotations.IsDefault) { return null; } @@ -365,12 +365,12 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) foreach (var param in sig.Parameters) { var (transformedParamType, paramTransformed) = handle(ref this, param.RefKind, param.RefCustomModifiers, param.TypeWithAnnotations); - if (transformedParamType is null) + if (transformedParamType.IsDefault) { return null; } - paramsBuilder.Add(transformedParamType.Value); + paramsBuilder.Add(transformedParamType); paramsTransformed |= paramTransformed; } @@ -385,7 +385,7 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) if (madeChanges) { - return type.SubstituteTypeSymbol(transformedReturnWithAnnotations.Value, transformedParameters, + return type.SubstituteTypeSymbol(transformedReturnWithAnnotations, transformedParameters, refCustomModifiers: default, paramRefCustomModifiers: default); } else @@ -393,19 +393,19 @@ private PointerTypeSymbol TransformPointerType(PointerTypeSymbol pointerType) return type; } - static (TypeWithAnnotations?, bool madeChanges) handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray refCustomModifiers, TypeWithAnnotations typeWithAnnotations) + static (TypeWithAnnotations, bool madeChanges) handle(ref DynamicTypeDecoder decoder, RefKind refKind, ImmutableArray refCustomModifiers, TypeWithAnnotations typeWithAnnotations) { if (!decoder.HandleCustomModifiers(refCustomModifiers.Length) || !decoder.HandleRefKind(refKind) || !decoder.HandleCustomModifiers(typeWithAnnotations.CustomModifiers.Length)) { - return (null, false); + return (default, false); } var transformedType = decoder.TransformType(typeWithAnnotations.Type); if (transformedType is null) { - return (null, false); + return (default, false); } if (transformedType.Equals(typeWithAnnotations.Type, TypeCompareKind.ConsiderEverything))