From db133b4c1953d665de76161dedcbb7dc6c0fb462 Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Fri, 6 Jan 2017 13:26:30 -0800 Subject: [PATCH 1/3] Emit forwarded types in a deterministic order Emit forwarded types in a deterministic order --- .../CSharp/Portable/Emitter/Model/PEModuleBuilder.cs | 5 ++++- src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs index 4b0555b8f9936..ae9e17fcbb1ae 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs @@ -557,7 +557,10 @@ private static void GetForwardedTypes( // (type, index of the parent exported type in builder, or -1 if the type is a top-level type) var stack = ArrayBuilder<(NamedTypeSymbol type, int parentIndex)>.GetInstance(); - foreach (NamedTypeSymbol forwardedType in wellKnownAttributeData.ForwardedTypes) + // Hashset enumeration is not guaranteed to be deterministic. Emitting in the order of fully qualified names. + var orderedForwardedTypes = wellKnownAttributeData.ForwardedTypes.OrderBy(t => t.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)); + + foreach (NamedTypeSymbol forwardedType in orderedForwardedTypes) { NamedTypeSymbol originalDefinition = forwardedType.OriginalDefinition; Debug.Assert((object)originalDefinition.ContainingType == null, "How did a nested type get forwarded?"); diff --git a/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb b/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb index 3956dd8b14663..efd0898ed1c07 100644 --- a/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb +++ b/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb @@ -517,7 +517,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit ' (type, index of the parent exported type in builder, or -1 if the type is a top-level type) Dim stack = ArrayBuilder(Of (type As NamedTypeSymbol, parentIndex As Integer)).GetInstance() - For Each forwardedType As NamedTypeSymbol In wellKnownAttributeData.ForwardedTypes + ' Hashset enumeration Is Not guaranteed to be deterministic. Emitting in the order of fully qualified names. + Dim orderedForwardedTypes = wellKnownAttributeData.ForwardedTypes.OrderBy(Function(t) t.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)) + + For Each forwardedType As NamedTypeSymbol In orderedForwardedTypes Dim originalDefinition As NamedTypeSymbol = forwardedType.OriginalDefinition Debug.Assert(originalDefinition.ContainingType Is Nothing, "How did a nested type get forwarded?") From 94a1519181105dc331cb6616e518bfbbbd84151f Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Fri, 6 Jan 2017 15:53:46 -0800 Subject: [PATCH 2/3] Added tests --- .../Test/Emit/Emit/DeterministicTests.cs | 49 +++++++++++++++ .../Test/Emit/Emit/DeterministicTests.vb | 62 +++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs index 8af876c9373f5..741ad8253a2d0 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs @@ -12,6 +12,7 @@ using Roslyn.Test.Utilities; using Microsoft.CodeAnalysis.Emit; using System.Diagnostics; +using System.Text; namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit { @@ -184,6 +185,54 @@ public void TestWriteOnlyStream() compilation.Emit(output); } + [Fact, WorkItem(11990, "https://github.com/dotnet/roslyn/issues/11990")] + public void ForwardedTypesAreEmmittedInADeterministicOrder() + { + const int generatedTypes = 100; + + var forwardedToCode = new StringBuilder(); + forwardedToCode.AppendLine("namespace ForwardedToNamespace {"); + for (var i = 1; i <= generatedTypes; i++) + { + forwardedToCode.AppendLine($"public class T{i} {{}}"); + } + forwardedToCode.AppendLine("}"); + + var forwardedToCompilation = CreateCompilation(forwardedToCode.ToString()); + var forwardedToReference = new CSharpCompilationReference(forwardedToCompilation); + + IEnumerable compileAndGetExportedTypes() + { + var forwardingCode = new StringBuilder(); + forwardingCode.AppendLine("using System.Runtime.CompilerServices;"); + for (var i = 1; i <= generatedTypes; i++) + { + forwardingCode.AppendLine($"[assembly: TypeForwardedTo(typeof(ForwardedToNamespace.T{i}))]"); + } + + var forwardingCompilation = CreateCompilationWithMscorlib(forwardingCode.ToString(), new MetadataReference[] { forwardedToReference }); + + using (var stream = forwardingCompilation.EmitToStream()) + { + using (var block = ModuleMetadata.CreateFromStream(stream)) + { + foreach (var handle in block.MetadataReader.ExportedTypes) + { + var type = block.MetadataReader.GetExportedType(handle); + var typeName = block.MetadataReader.GetString(type.Name); + yield return typeName; + } + } + } + } + + var baseline = compileAndGetExportedTypes().ToArray(); + Assert.Equal(generatedTypes, baseline.Length); + + var reference = compileAndGetExportedTypes().ToArray(); + Assert.Equal(baseline, reference); + } + [Fact] public void TestPartialPartsDeterministic() { diff --git a/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb b/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb index 3ca0c257d78ce..6112c28472072 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb @@ -7,6 +7,7 @@ Imports System.Collections.Immutable Imports System.Threading Imports Microsoft.CodeAnalysis.Test.Utilities Imports Roslyn.Test.Utilities +Imports System.Text Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests.Emit @@ -131,6 +132,67 @@ Partial.c = 3 Next End Sub + + + Public Sub ForwardedTypesAreEmmittedInADeterministicOrder() + Const generatedTypes = 100 + + ' VBC doesn't recognize type forwards in VB source code. Because of that, + ' this test generates types as a C# library (1), then generates a C# netmodule (2) that + ' contains the type forwards that forwards to reference (1), then generates an empty VB + ' library (3) that contains (2) and references (1). + + Dim forwardedToCode = New StringBuilder() + forwardedToCode.AppendLine("namespace ForwardedToNamespace {") + For i = 1 To generatedTypes + forwardedToCode.AppendLine($"public class T{i} {{}}") + Next + forwardedToCode.AppendLine("}") + + Dim forwardedToCompilation = CreateCSharpCompilation( + forwardedToCode.ToString(), + compilationOptions:=New CSharp.CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + + Dim compileAndGetExportedTypes As Func(Of List(Of String)) = + Function() + Dim forwardingCode = New StringBuilder() + forwardingCode.AppendLine("using System.Runtime.CompilerServices;") + For i = 1 To generatedTypes + forwardingCode.AppendLine($"[assembly: TypeForwardedTo(typeof(ForwardedToNamespace.T{i}))]") + Next + + Dim forwardingNetModule = CreateCSharpCompilation( + "ForwardingAssembly", + forwardingCode.ToString(), + compilationOptions:=New CSharp.CSharpCompilationOptions(OutputKind.NetModule), + referencedAssemblies:={MscorlibRef, SystemRef, forwardedToCompilation.EmitToImageReference()}) + + Dim forwardingCompilation = CreateCompilationWithMscorlib( + assemblyName:="ForwardingAssembly", + source:=String.Empty, + options:=New VisualBasicCompilationOptions(OutputKind.DynamicallyLinkedLibrary), + references:={forwardingNetModule.EmitToImageReference(), forwardedToCompilation.EmitToImageReference()}) + + Using stream = forwardingCompilation.EmitToStream() + Using block = ModuleMetadata.CreateFromStream(stream) + Dim typeNames = New List(Of String)(block.MetadataReader.ExportedTypes.Count) + For Each handle In block.MetadataReader.ExportedTypes + Dim Type = block.MetadataReader.GetExportedType(handle) + Dim TypeName = block.MetadataReader.GetString(Type.Name) + typeNames.Add(TypeName) + Next + Return typeNames + End Using + End Using + End Function + + Dim baseline = compileAndGetExportedTypes().ToArray() + Assert.Equal(generatedTypes, baseline.Length) + + Dim reference = compileAndGetExportedTypes().ToArray() + Assert.Equal(baseline, reference) + End Sub + Public Sub TestInterfacesPartialClassDeterminism() ' When we have parts of a class in different trees, From 4bc408f44f4539dea6f0938fe899ca647e55e376 Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Wed, 11 Jan 2017 14:58:16 -0800 Subject: [PATCH 3/3] Addressed CR comments --- .../Test/Emit/Emit/DeterministicTests.cs | 98 ++++++++++------ .../Portable/Emit/PEModuleBuilder.vb | 2 +- .../Test/Emit/Emit/DeterministicTests.vb | 108 +++++++++++------- .../Portable/Metadata/MetadataValidation.cs | 12 ++ 4 files changed, 140 insertions(+), 80 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs index 741ad8253a2d0..7af8a4edd7fee 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs @@ -12,7 +12,6 @@ using Roslyn.Test.Utilities; using Microsoft.CodeAnalysis.Emit; using System.Diagnostics; -using System.Text; namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit { @@ -186,51 +185,74 @@ public void TestWriteOnlyStream() } [Fact, WorkItem(11990, "https://github.com/dotnet/roslyn/issues/11990")] - public void ForwardedTypesAreEmmittedInADeterministicOrder() + public void ForwardedTypesAreEmittedInADeterministicOrder() { - const int generatedTypes = 100; - - var forwardedToCode = new StringBuilder(); - forwardedToCode.AppendLine("namespace ForwardedToNamespace {"); - for (var i = 1; i <= generatedTypes; i++) - { - forwardedToCode.AppendLine($"public class T{i} {{}}"); - } - forwardedToCode.AppendLine("}"); - - var forwardedToCompilation = CreateCompilation(forwardedToCode.ToString()); + var forwardedToCode = @" +namespace Namespace2 { + public class GenericType1 {} + public class GenericType3 {} + public class GenericType2 {} +} +namespace Namespace1 { + public class Type3 {} + public class Type2 {} + public class Type1 {} +} +namespace Namespace4 { + namespace Embedded { + public class Type2 {} + public class Type1 {} + } +} +namespace Namespace3 { + public class GenericType {} + public class GenericType {} + public class GenericType {} +} +"; + var forwardedToCompilation = CreateCompilation(forwardedToCode); var forwardedToReference = new CSharpCompilationReference(forwardedToCompilation); - IEnumerable compileAndGetExportedTypes() - { - var forwardingCode = new StringBuilder(); - forwardingCode.AppendLine("using System.Runtime.CompilerServices;"); - for (var i = 1; i <= generatedTypes; i++) - { - forwardingCode.AppendLine($"[assembly: TypeForwardedTo(typeof(ForwardedToNamespace.T{i}))]"); - } + var forwardingCode = @" +using System.Runtime.CompilerServices; +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType1))] +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType3))] +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType2))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type3))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type2))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type1))] +[assembly: TypeForwardedTo(typeof(Namespace4.Embedded.Type2))] +[assembly: TypeForwardedTo(typeof(Namespace4.Embedded.Type1))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +"; - var forwardingCompilation = CreateCompilationWithMscorlib(forwardingCode.ToString(), new MetadataReference[] { forwardedToReference }); + var forwardingCompilation = CreateCompilationWithMscorlib(forwardingCode, new MetadataReference[] { forwardedToReference }); - using (var stream = forwardingCompilation.EmitToStream()) + var sortedFullNames = new string[] + { + "Namespace1.Type1", + "Namespace1.Type2", + "Namespace1.Type3", + "Namespace2.GenericType1`1", + "Namespace2.GenericType2`1", + "Namespace2.GenericType3`1", + "Namespace3.GenericType", + "Namespace3.GenericType`1", + "Namespace3.GenericType`2", + "Namespace4.Embedded.Type1", + "Namespace4.Embedded.Type2" + }; + + using (var stream = forwardingCompilation.EmitToStream()) + { + using (var block = ModuleMetadata.CreateFromStream(stream)) { - using (var block = ModuleMetadata.CreateFromStream(stream)) - { - foreach (var handle in block.MetadataReader.ExportedTypes) - { - var type = block.MetadataReader.GetExportedType(handle); - var typeName = block.MetadataReader.GetString(type.Name); - yield return typeName; - } - } + var metadataFullNames = MetadataValidation.GetExportedTypesFullNames(block.MetadataReader); + Assert.Equal(sortedFullNames, metadataFullNames); } } - - var baseline = compileAndGetExportedTypes().ToArray(); - Assert.Equal(generatedTypes, baseline.Length); - - var reference = compileAndGetExportedTypes().ToArray(); - Assert.Equal(baseline, reference); } [Fact] diff --git a/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb b/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb index efd0898ed1c07..b0424cf0b3bff 100644 --- a/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb +++ b/src/Compilers/VisualBasic/Portable/Emit/PEModuleBuilder.vb @@ -517,7 +517,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit ' (type, index of the parent exported type in builder, or -1 if the type is a top-level type) Dim stack = ArrayBuilder(Of (type As NamedTypeSymbol, parentIndex As Integer)).GetInstance() - ' Hashset enumeration Is Not guaranteed to be deterministic. Emitting in the order of fully qualified names. + ' Hashset enumeration is not guaranteed to be deterministic. Emitting in the order of fully qualified names. Dim orderedForwardedTypes = wellKnownAttributeData.ForwardedTypes.OrderBy(Function(t) t.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)) For Each forwardedType As NamedTypeSymbol In orderedForwardedTypes diff --git a/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb b/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb index 6112c28472072..9b3287bf07ef0 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Emit/DeterministicTests.vb @@ -7,7 +7,6 @@ Imports System.Collections.Immutable Imports System.Threading Imports Microsoft.CodeAnalysis.Test.Utilities Imports Roslyn.Test.Utilities -Imports System.Text Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests.Emit @@ -134,63 +133,90 @@ Partial.c = 3 - Public Sub ForwardedTypesAreEmmittedInADeterministicOrder() - Const generatedTypes = 100 - + Public Sub ForwardedTypesAreEmittedInADeterministicOrder() ' VBC doesn't recognize type forwards in VB source code. Because of that, ' this test generates types as a C# library (1), then generates a C# netmodule (2) that ' contains the type forwards that forwards to reference (1), then generates an empty VB ' library (3) that contains (2) and references (1). - Dim forwardedToCode = New StringBuilder() - forwardedToCode.AppendLine("namespace ForwardedToNamespace {") - For i = 1 To generatedTypes - forwardedToCode.AppendLine($"public class T{i} {{}}") - Next - forwardedToCode.AppendLine("}") - + Dim forwardedToCode = " +namespace Namespace2 { + public class GenericType1 {} + public class GenericType3 {} + public class GenericType2 {} +} +namespace Namespace1 { + public class Type3 {} + public class Type2 {} + public class Type1 {} +} +namespace Namespace4 { + namespace Embedded { + public class Type2 {} + public class Type1 {} + } +} +namespace Namespace3 { + public class GenericType {} + public class GenericType {} + public class GenericType {} +} +" Dim forwardedToCompilation = CreateCSharpCompilation( - forwardedToCode.ToString(), + forwardedToCode, compilationOptions:=New CSharp.CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) - Dim compileAndGetExportedTypes As Func(Of List(Of String)) = - Function() - Dim forwardingCode = New StringBuilder() - forwardingCode.AppendLine("using System.Runtime.CompilerServices;") - For i = 1 To generatedTypes - forwardingCode.AppendLine($"[assembly: TypeForwardedTo(typeof(ForwardedToNamespace.T{i}))]") - Next + Dim forwardedToReference = forwardedToCompilation.EmitToImageReference() - Dim forwardingNetModule = CreateCSharpCompilation( + Dim forwardingCode = " +using System.Runtime.CompilerServices; +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType1))] +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType3))] +[assembly: TypeForwardedTo(typeof(Namespace2.GenericType2))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type3))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type2))] +[assembly: TypeForwardedTo(typeof(Namespace1.Type1))] +[assembly: TypeForwardedTo(typeof(Namespace4.Embedded.Type2))] +[assembly: TypeForwardedTo(typeof(Namespace4.Embedded.Type1))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +[assembly: TypeForwardedTo(typeof(Namespace3.GenericType))] +" + Dim forwardingNetModule = CreateCSharpCompilation( "ForwardingAssembly", - forwardingCode.ToString(), + forwardingCode, compilationOptions:=New CSharp.CSharpCompilationOptions(OutputKind.NetModule), - referencedAssemblies:={MscorlibRef, SystemRef, forwardedToCompilation.EmitToImageReference()}) + referencedAssemblies:={MscorlibRef, SystemRef, forwardedToReference}) - Dim forwardingCompilation = CreateCompilationWithMscorlib( + Dim forwardingNetModuleReference = forwardingNetModule.EmitToImageReference() + + Dim forwardingCompilation = CreateCompilationWithMscorlib( assemblyName:="ForwardingAssembly", source:=String.Empty, options:=New VisualBasicCompilationOptions(OutputKind.DynamicallyLinkedLibrary), - references:={forwardingNetModule.EmitToImageReference(), forwardedToCompilation.EmitToImageReference()}) - - Using stream = forwardingCompilation.EmitToStream() - Using block = ModuleMetadata.CreateFromStream(stream) - Dim typeNames = New List(Of String)(block.MetadataReader.ExportedTypes.Count) - For Each handle In block.MetadataReader.ExportedTypes - Dim Type = block.MetadataReader.GetExportedType(handle) - Dim TypeName = block.MetadataReader.GetString(Type.Name) - typeNames.Add(TypeName) - Next - Return typeNames - End Using - End Using - End Function + references:={forwardingNetModuleReference, forwardedToReference}) - Dim baseline = compileAndGetExportedTypes().ToArray() - Assert.Equal(generatedTypes, baseline.Length) + Dim sortedFullNames = + { + "Namespace1.Type1", + "Namespace1.Type2", + "Namespace1.Type3", + "Namespace2.GenericType1`1", + "Namespace2.GenericType2`1", + "Namespace2.GenericType3`1", + "Namespace3.GenericType", + "Namespace3.GenericType`1", + "Namespace3.GenericType`2", + "Namespace4.Embedded.Type1", + "Namespace4.Embedded.Type2" + } - Dim reference = compileAndGetExportedTypes().ToArray() - Assert.Equal(baseline, reference) + Using stream = forwardingCompilation.EmitToStream() + Using block = ModuleMetadata.CreateFromStream(stream) + Dim metadataFullNames = MetadataValidation.GetExportedTypesFullNames(block.MetadataReader) + Assert.Equal(sortedFullNames, metadataFullNames) + End Using + End Using End Sub diff --git a/src/Test/Utilities/Portable/Metadata/MetadataValidation.cs b/src/Test/Utilities/Portable/Metadata/MetadataValidation.cs index 4b782b8a6dbaf..67203dcb1554c 100644 --- a/src/Test/Utilities/Portable/Metadata/MetadataValidation.cs +++ b/src/Test/Utilities/Portable/Metadata/MetadataValidation.cs @@ -132,5 +132,17 @@ internal static IEnumerable GetFullTypeNames(MetadataReader metadataRead yield return (ns.Length == 0) ? name : (ns + "." + name); } } + + internal static IEnumerable GetExportedTypesFullNames(MetadataReader metadataReader) + { + foreach (var typeDefHandle in metadataReader.ExportedTypes) + { + var typeDef = metadataReader.GetExportedType(typeDefHandle); + var ns = metadataReader.GetString(typeDef.Namespace); + var name = metadataReader.GetString(typeDef.Name); + + yield return (ns.Length == 0) ? name : (ns + "." + name); + } + } } }