From 9b31f90a140518c7f39ed869a4f7e21dd3af04e3 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 7 May 2020 14:32:26 -0700 Subject: [PATCH 1/4] Add SymbolMatcher support for function pointers. --- .../EditAndContinue/CSharpSymbolMatcher.cs | 140 ++++++++++++++++++ .../EditAndContinue/SymbolMatcherTests.cs | 131 ++++++++++++++++ 2 files changed, 271 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs index 737be6ba5e658..3f933e0b47c6e 100644 --- a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs +++ b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs @@ -561,6 +561,57 @@ public override Symbol VisitPointerType(PointerTypeSymbol symbol) return new PointerTypeSymbol(symbol.PointedAtTypeWithAnnotations.WithTypeAndModifiers(otherPointedAtType, otherModifiers)); } + public override Symbol VisitFunctionPointerType(FunctionPointerTypeSymbol symbol) + { + var sig = symbol.Signature; + + var otherReturnType = (TypeSymbol)Visit(sig.ReturnType); + var translationFailed = otherReturnType is null; + var otherRefCustomModifiers = VisitCustomModifiers(sig.RefCustomModifiers); + var otherReturnTypeWithAnnotations = sig.ReturnTypeWithAnnotations.WithTypeAndModifiers(otherReturnType, VisitCustomModifiers(sig.ReturnTypeWithAnnotations.CustomModifiers)); + + var otherParameterTypes = ImmutableArray.Empty; + ImmutableArray> otherParamRefCustomModifiers = default; + + if (sig.ParameterCount > 0) + { + var otherParamsBuilder = ArrayBuilder.GetInstance(sig.ParameterCount); + var otherParamRefCustomModifiersBuilder = ArrayBuilder>.GetInstance(sig.ParameterCount); + + foreach (var param in sig.Parameters) + { + var otherType = (TypeSymbol)Visit(param.Type); + if (otherType is null) + { + translationFailed = true; + } + + otherParamRefCustomModifiersBuilder.Add(VisitCustomModifiers(param.RefCustomModifiers)); + otherParamsBuilder.Add(param.TypeWithAnnotations.WithTypeAndModifiers(otherType, VisitCustomModifiers(param.TypeWithAnnotations.CustomModifiers))); + } + + if (translationFailed) + { + otherParamsBuilder.Free(); + otherParamRefCustomModifiersBuilder.Free(); + } + else + { + otherParameterTypes = otherParamsBuilder.ToImmutableAndFree(); + otherParamRefCustomModifiers = otherParamRefCustomModifiersBuilder.ToImmutableAndFree(); + } + } + + if (translationFailed) + { + return null; + } + else + { + return symbol.SubstituteTypeSymbol(otherReturnTypeWithAnnotations, otherParameterTypes, otherRefCustomModifiers, otherParamRefCustomModifiers); + } + } + public override Symbol VisitProperty(PropertySymbol symbol) { return this.VisitNamedTypeMember(symbol, ArePropertiesEqual); @@ -736,6 +787,63 @@ private bool ArePointerTypesEqual(PointerTypeSymbol type, PointerTypeSymbol othe return AreTypesEqual(type.PointedAtType, other.PointedAtType); } + private bool AreFunctionPointerTypesEqual(FunctionPointerTypeSymbol type, FunctionPointerTypeSymbol other) + { + var sig = type.Signature; + var otherSig = other.Signature; + + if (!isEqual(sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations, + sig.RefKind, otherSig.RefKind, + sig.RefCustomModifiers, otherSig.RefCustomModifiers, + allowOut: false)) + { + return false; + } + + if (sig.ParameterCount != otherSig.ParameterCount) + { + return false; + } + + for (int i = 0; i < sig.ParameterCount; i++) + { + var param = sig.Parameters[i]; + var otherParam = otherSig.Parameters[i]; + if (!isEqual(param.TypeWithAnnotations, otherParam.TypeWithAnnotations, + param.RefKind, otherParam.RefKind, + param.RefCustomModifiers, otherParam.RefCustomModifiers, + allowOut: true)) + { + return false; + } + } + + return true; + + bool isEqual(TypeWithAnnotations type, TypeWithAnnotations otherType, RefKind refKind, RefKind otherRefKind, ImmutableArray refCustomModifiers, ImmutableArray otherRefCustomModifiers, bool allowOut) + { + Debug.Assert(type.CustomModifiers.IsEmpty); + Debug.Assert(otherType.CustomModifiers.IsEmpty); + Debug.Assert(verifyRefModifiers(refCustomModifiers, refKind, allowOut)); + Debug.Assert(verifyRefModifiers(otherRefCustomModifiers, otherRefKind, allowOut)); + + return refKind != otherRefKind || !AreTypesEqual(type.Type, otherType.Type); + } + + static bool verifyRefModifiers(ImmutableArray modifiers, RefKind refKind, bool allowOut) + { + Debug.Assert(RefKind.RefReadOnly == RefKind.In); + switch (refKind) + { + case RefKind.RefReadOnly: + case RefKind.Out when allowOut: + return modifiers.Length == 1; + default: + return modifiers.IsDefaultOrEmpty; + } + } + } + private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other) { Debug.Assert(StringOrdinalComparer.Equals(property.MetadataName, other.MetadataName)); @@ -781,6 +889,9 @@ private bool AreTypesEqual(TypeSymbol type, TypeSymbol other) case SymbolKind.PointerType: return ArePointerTypesEqual((PointerTypeSymbol)type, (PointerTypeSymbol)other); + case SymbolKind.FunctionPointer: + return AreFunctionPointerTypesEqual((FunctionPointerTypeSymbol)type, (FunctionPointerTypeSymbol)other); + case SymbolKind.NamedType: case SymbolKind.ErrorType: return AreNamedTypesEqual((NamedTypeSymbol)type, (NamedTypeSymbol)other); @@ -915,6 +1026,35 @@ public override Symbol VisitPointerType(PointerTypeSymbol symbol) return new PointerTypeSymbol(symbol.PointedAtTypeWithAnnotations.WithTypeAndModifiers(translatedPointedAtType, translatedModifiers)); } + public override Symbol VisitFunctionPointerType(FunctionPointerTypeSymbol symbol) + { + var sig = symbol.Signature; + var translatedReturnType = (TypeSymbol)Visit(sig.ReturnType); + var translatedReturnTypeWithAnnotations = sig.ReturnTypeWithAnnotations.WithTypeAndModifiers(translatedReturnType, VisitCustomModifiers(sig.ReturnTypeWithAnnotations.CustomModifiers)); + var translatedRefCustomModifiers = VisitCustomModifiers(sig.RefCustomModifiers); + + var translatedParameterTypes = ImmutableArray.Empty; + ImmutableArray> translatedParamRefCustomModifiers = default; + + if (sig.ParameterCount > 0) + { + var translatedParamsBuilder = ArrayBuilder.GetInstance(sig.ParameterCount); + var translatedParamRefCustomModifiersBuilder = ArrayBuilder>.GetInstance(sig.ParameterCount); + + foreach (var param in sig.Parameters) + { + var translatedParamType = (TypeSymbol)Visit(param.Type); + translatedParamsBuilder.Add(param.TypeWithAnnotations.WithTypeAndModifiers(translatedParamType, VisitCustomModifiers(param.TypeWithAnnotations.CustomModifiers))); + translatedParamRefCustomModifiersBuilder.Add(VisitCustomModifiers(param.RefCustomModifiers)); + } + + translatedParameterTypes = translatedParamsBuilder.ToImmutableAndFree(); + translatedParamRefCustomModifiers = translatedParamRefCustomModifiersBuilder.ToImmutableAndFree(); + } + + return symbol.SubstituteTypeSymbol(translatedReturnTypeWithAnnotations, translatedParameterTypes, translatedRefCustomModifiers, translatedParamRefCustomModifiers); + } + public override Symbol VisitTypeParameter(TypeParameterSymbol symbol) { return symbol; diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs index 4086200c04444..4484a20ca05b5 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs @@ -1272,5 +1272,136 @@ event Action F { add { } remove { } } Assert.Same(e0, matcher.MapDefinition(e1)); Assert.Same(f0, matcher.MapDefinition(f1)); } + + [Fact] + public void FunctionPointerMembersTranslated() + { + var source = @" +unsafe class C +{ + delegate* f1; + delegate* f2; + delegate* f3; + delegate* f4; + delegate* f5; + delegate* f6; + delegate* f7; +} +class D {} +"; + + var compilation0 = CreateCompilation(source, options: TestOptions.UnsafeDebugDll, parseOptions: TestOptions.RegularPreview); + var compilation1 = compilation0.WithSource(source); + + var matcher = new CSharpSymbolMatcher( + null, + compilation1.SourceAssembly, + default, + compilation0.SourceAssembly, + default, + null); + + for (int i = 1; i <= 7; i++) + { + var f_0 = compilation0.GetMember($"C.f{i}"); + var f_1 = compilation1.GetMember($"C.f{i}"); + + Assert.Same(f_0, matcher.MapDefinition(f_1)); + } + } + + [Theory] + [InlineData("C", "void")] + [InlineData("C", "object")] + [InlineData("C", "ref C")] + [InlineData("C", "ref readonly C")] + [InlineData("ref C", "ref readonly C")] + public void FunctionPointerMembers_ReturnMismatch(string return1, string return2) + { + var source1 = $@" +unsafe class C +{{ + delegate* f1; +}}"; + + var source2 = $@" +unsafe class C +{{ + delegate* f1; +}}"; + + var compilation0 = CreateCompilation(source1, options: TestOptions.UnsafeDebugDll, parseOptions: TestOptions.RegularPreview); + var compilation1 = compilation0.WithSource(source2); + + var matcher = new CSharpSymbolMatcher( + null, + compilation1.SourceAssembly, + default, + compilation0.SourceAssembly, + default, + null); + + var f_1 = compilation1.GetMember($"C.f1"); + + Assert.Null(matcher.MapDefinition(f_1)); + } + + [Theory] + [InlineData("C", "object")] + [InlineData("C", "ref C")] + [InlineData("C", "out C")] + [InlineData("C", "in C")] + [InlineData("ref C", "out C")] + [InlineData("ref C", "in C")] + [InlineData("out C", "in C")] + [InlineData("C, C", "C")] + public void FunctionPointerMembers_ParamMismatch(string param1, string param2) + { + var source1 = $@" +unsafe class C +{{ + delegate*<{param1}, C, void>* f1; +}}"; + + var source2 = $@" +unsafe class C +{{ + delegate*<{param2}, C, void>* f1; +}}"; + + verify(source1, source2); + + source1 = $@" +unsafe class C +{{ + delegate* f1; +}}"; + + source2 = $@" +unsafe class C +{{ + delegate* f1; +}}"; + + verify(source1, source2); + + static void verify(string source1, string source2) + { + var compilation0 = CreateCompilation(source1, options: TestOptions.UnsafeDebugDll, parseOptions: TestOptions.RegularPreview); + var compilation1 = compilation0.WithSource(source2); + + var matcher = new CSharpSymbolMatcher( + null, + compilation1.SourceAssembly, + default, + compilation0.SourceAssembly, + default, + null); + + var f_1 = compilation1.GetMember($"C.f1"); + + Assert.Null(matcher.MapDefinition(f_1)); + } + } } } From ad6efb68d07104142b3508ff547de87518031f6a Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 14 May 2020 11:25:13 -0700 Subject: [PATCH 2/4] Simplify SymbolMatcher implementations. --- .../EditAndContinue/CSharpSymbolMatcher.cs | 84 ++++++++----------- .../EditAndContinue/SymbolMatcherTests.cs | 1 - 2 files changed, 35 insertions(+), 50 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs index 3f933e0b47c6e..0c5e9ad6a48d1 100644 --- a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs +++ b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs @@ -566,7 +566,11 @@ public override Symbol VisitFunctionPointerType(FunctionPointerTypeSymbol symbol var sig = symbol.Signature; var otherReturnType = (TypeSymbol)Visit(sig.ReturnType); - var translationFailed = otherReturnType is null; + if (otherReturnType is null) + { + return null; + } + var otherRefCustomModifiers = VisitCustomModifiers(sig.RefCustomModifiers); var otherReturnTypeWithAnnotations = sig.ReturnTypeWithAnnotations.WithTypeAndModifiers(otherReturnType, VisitCustomModifiers(sig.ReturnTypeWithAnnotations.CustomModifiers)); @@ -583,33 +587,20 @@ public override Symbol VisitFunctionPointerType(FunctionPointerTypeSymbol symbol var otherType = (TypeSymbol)Visit(param.Type); if (otherType is null) { - translationFailed = true; + otherParamsBuilder.Free(); + otherParamRefCustomModifiersBuilder.Free(); + return null; } otherParamRefCustomModifiersBuilder.Add(VisitCustomModifiers(param.RefCustomModifiers)); otherParamsBuilder.Add(param.TypeWithAnnotations.WithTypeAndModifiers(otherType, VisitCustomModifiers(param.TypeWithAnnotations.CustomModifiers))); } - if (translationFailed) - { - otherParamsBuilder.Free(); - otherParamRefCustomModifiersBuilder.Free(); - } - else - { - otherParameterTypes = otherParamsBuilder.ToImmutableAndFree(); - otherParamRefCustomModifiers = otherParamRefCustomModifiersBuilder.ToImmutableAndFree(); - } + otherParameterTypes = otherParamsBuilder.ToImmutableAndFree(); + otherParamRefCustomModifiers = otherParamRefCustomModifiersBuilder.ToImmutableAndFree(); } - if (translationFailed) - { - return null; - } - else - { - return symbol.SubstituteTypeSymbol(otherReturnTypeWithAnnotations, otherParameterTypes, otherRefCustomModifiers, otherParamRefCustomModifiers); - } + return symbol.SubstituteTypeSymbol(otherReturnTypeWithAnnotations, otherParameterTypes, otherRefCustomModifiers, otherParamRefCustomModifiers); } public override Symbol VisitProperty(PropertySymbol symbol) @@ -792,43 +783,38 @@ private bool AreFunctionPointerTypesEqual(FunctionPointerTypeSymbol type, Functi var sig = type.Signature; var otherSig = other.Signature; - if (!isEqual(sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations, - sig.RefKind, otherSig.RefKind, - sig.RefCustomModifiers, otherSig.RefCustomModifiers, - allowOut: false)) + ValidateFunctionPointerParamOrReturn( + sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations, + sig.RefKind, otherSig.RefKind, + sig.RefCustomModifiers, otherSig.RefCustomModifiers, + allowOut: false); + if (sig.RefKind != otherSig.RefKind || !AreTypesEqual(sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations)) { return false; } - if (sig.ParameterCount != otherSig.ParameterCount) - { - return false; - } + return sig.Parameters.SequenceEqual(otherSig.Parameters, AreFunctionPointerParametersEqual); + } - for (int i = 0; i < sig.ParameterCount; i++) - { - var param = sig.Parameters[i]; - var otherParam = otherSig.Parameters[i]; - if (!isEqual(param.TypeWithAnnotations, otherParam.TypeWithAnnotations, - param.RefKind, otherParam.RefKind, - param.RefCustomModifiers, otherParam.RefCustomModifiers, - allowOut: true)) - { - return false; - } - } + private bool AreFunctionPointerParametersEqual(ParameterSymbol param, ParameterSymbol otherParam) + { + ValidateFunctionPointerParamOrReturn( + param.TypeWithAnnotations, otherParam.TypeWithAnnotations, + param.RefKind, otherParam.RefKind, + param.RefCustomModifiers, otherParam.RefCustomModifiers, + allowOut: false); - return true; + return param.RefKind == otherParam.RefKind && AreTypesEqual(param.TypeWithAnnotations, otherParam.TypeWithAnnotations); + } - bool isEqual(TypeWithAnnotations type, TypeWithAnnotations otherType, RefKind refKind, RefKind otherRefKind, ImmutableArray refCustomModifiers, ImmutableArray otherRefCustomModifiers, bool allowOut) - { - Debug.Assert(type.CustomModifiers.IsEmpty); - Debug.Assert(otherType.CustomModifiers.IsEmpty); - Debug.Assert(verifyRefModifiers(refCustomModifiers, refKind, allowOut)); - Debug.Assert(verifyRefModifiers(otherRefCustomModifiers, otherRefKind, allowOut)); + [Conditional("DEBUG")] + private void ValidateFunctionPointerParamOrReturn(TypeWithAnnotations type, TypeWithAnnotations otherType, RefKind refKind, RefKind otherRefKind, ImmutableArray refCustomModifiers, ImmutableArray otherRefCustomModifiers, bool allowOut) + { - return refKind != otherRefKind || !AreTypesEqual(type.Type, otherType.Type); - } + Debug.Assert(type.CustomModifiers.IsEmpty); + Debug.Assert(otherType.CustomModifiers.IsEmpty); + Debug.Assert(verifyRefModifiers(refCustomModifiers, refKind, allowOut)); + Debug.Assert(verifyRefModifiers(otherRefCustomModifiers, otherRefKind, allowOut)); static bool verifyRefModifiers(ImmutableArray modifiers, RefKind refKind, bool allowOut) { diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs index 4484a20ca05b5..5b3965d72f865 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/SymbolMatcherTests.cs @@ -1287,7 +1287,6 @@ unsafe class C delegate* f6; delegate* f7; } -class D {} "; var compilation0 = CreateCompilation(source, options: TestOptions.UnsafeDebugDll, parseOptions: TestOptions.RegularPreview); From e17bc1c936e1ab10a8576b9be166968e43bd1556 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Thu, 14 May 2020 12:48:55 -0700 Subject: [PATCH 3/4] PR Feedback --- .../EditAndContinue/CSharpSymbolMatcher.cs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs index 0c5e9ad6a48d1..eab5df5602d26 100644 --- a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs +++ b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs @@ -783,11 +783,8 @@ private bool AreFunctionPointerTypesEqual(FunctionPointerTypeSymbol type, Functi var sig = type.Signature; var otherSig = other.Signature; - ValidateFunctionPointerParamOrReturn( - sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations, - sig.RefKind, otherSig.RefKind, - sig.RefCustomModifiers, otherSig.RefCustomModifiers, - allowOut: false); + ValidateFunctionPointerParamOrReturn(sig.ReturnTypeWithAnnotations, sig.RefKind, sig.RefCustomModifiers, allowOut: false); + ValidateFunctionPointerParamOrReturn(otherSig.ReturnTypeWithAnnotations, otherSig.RefKind, otherSig.RefCustomModifiers, allowOut: false); if (sig.RefKind != otherSig.RefKind || !AreTypesEqual(sig.ReturnTypeWithAnnotations, otherSig.ReturnTypeWithAnnotations)) { return false; @@ -798,23 +795,17 @@ private bool AreFunctionPointerTypesEqual(FunctionPointerTypeSymbol type, Functi private bool AreFunctionPointerParametersEqual(ParameterSymbol param, ParameterSymbol otherParam) { - ValidateFunctionPointerParamOrReturn( - param.TypeWithAnnotations, otherParam.TypeWithAnnotations, - param.RefKind, otherParam.RefKind, - param.RefCustomModifiers, otherParam.RefCustomModifiers, - allowOut: false); + ValidateFunctionPointerParamOrReturn(param.TypeWithAnnotations, param.RefKind, param.RefCustomModifiers, allowOut: true); + ValidateFunctionPointerParamOrReturn(otherParam.TypeWithAnnotations, otherParam.RefKind, otherParam.RefCustomModifiers, allowOut: true); return param.RefKind == otherParam.RefKind && AreTypesEqual(param.TypeWithAnnotations, otherParam.TypeWithAnnotations); } [Conditional("DEBUG")] - private void ValidateFunctionPointerParamOrReturn(TypeWithAnnotations type, TypeWithAnnotations otherType, RefKind refKind, RefKind otherRefKind, ImmutableArray refCustomModifiers, ImmutableArray otherRefCustomModifiers, bool allowOut) + private static void ValidateFunctionPointerParamOrReturn(TypeWithAnnotations type, RefKind refKind, ImmutableArray refCustomModifiers, bool allowOut) { - Debug.Assert(type.CustomModifiers.IsEmpty); - Debug.Assert(otherType.CustomModifiers.IsEmpty); Debug.Assert(verifyRefModifiers(refCustomModifiers, refKind, allowOut)); - Debug.Assert(verifyRefModifiers(otherRefCustomModifiers, otherRefKind, allowOut)); static bool verifyRefModifiers(ImmutableArray modifiers, RefKind refKind, bool allowOut) { From f1fc86793891f1be82b4e89657faab8f8a698da4 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 15 May 2020 16:05:47 -0700 Subject: [PATCH 4/4] Remove unneeded Default check. --- .../Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs index eab5df5602d26..36eeb9360d607 100644 --- a/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs +++ b/src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs @@ -816,7 +816,7 @@ static bool verifyRefModifiers(ImmutableArray modifiers, RefKind case RefKind.Out when allowOut: return modifiers.Length == 1; default: - return modifiers.IsDefaultOrEmpty; + return modifiers.IsEmpty; } } }