From d77cda297f9190c9d16679277298f6d5230c549e Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Fri, 18 Sep 2015 10:31:36 -0700 Subject: [PATCH 1/6] Only add a type forward if the nested type is visible outside of the assembly. --- src/GenFacades/Program.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index ff5b2a82a7..a7992d5592 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -495,7 +495,8 @@ private void AddTypeForward(Assembly assembly, INamedTypeDefinition seedType) // but the runtime currently throws a TypeLoadException without explicit forwarders for the nested // types. foreach (var nestedType in seedType.NestedTypes.OrderBy(t => t.Name.Value)) - AddTypeForward(assembly, nestedType); + if (nestedType.IsVisibleOutsideAssembly()) + AddTypeForward(assembly, nestedType); } private void AddWin32VersionResource(string contractLocation, Assembly facade) From bbce670763ad91e2d6ba32f4ba75577f7ec9467a Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Fri, 18 Sep 2015 11:11:07 -0700 Subject: [PATCH 2/6] Fixing comment since we are now includding only the nested types that are visible outside the assembly. --- src/GenFacades/Program.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index a7992d5592..2c7d57096f 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -485,12 +485,7 @@ private void AddTypeForward(Assembly assembly, INamedTypeDefinition seedType) assembly.ExportedTypes = new List(); assembly.ExportedTypes.Add(alias); - // Recursively add forwarders for all nested types regardless of their accessibility. This is - // how the C# compiler emits type forwarders for nested types. We might not need them for - // nested types that are not visible outside the assembly, but it is safer to replicate how - // the C# compiler works. Plus, it helps when diffing the output from ildasm for facades - // that were built from source vs. those that were produced by this tool. - // + // Recursively add forwarders only for the nested types that are accessible outside of the assembly. // NOTE: Some design-time tools can resolve forwarded nested types with only the top-level forwarder, // but the runtime currently throws a TypeLoadException without explicit forwarders for the nested // types. From 1c92e352b1d2d8c58b3f2957c399088d1653775a Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Tue, 22 Sep 2015 17:01:45 -0700 Subject: [PATCH 3/6] Changing type forwards to be looped from the contract assembly instead of the seedtype. --- src/GenFacades/Program.cs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index 2c7d57096f..36cfb81a00 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -417,7 +417,7 @@ public Assembly GenerateFacade(IAssembly contractAssembly, IAssemblyReference se continue; } - AddTypeForward(assembly, seedType); + AddTypeForward(assembly, seedType, contractAssembly); } if (error) @@ -475,7 +475,7 @@ private static void TraceDuplicateSeedTypeError(string docId, IReadOnlyList(); assembly.ExportedTypes.Add(alias); - // Recursively add forwarders only for the nested types that are accessible outside of the assembly. + INamedTypeDefinition contractType = contractAssembly.GetAllTypes().Where(t => t.FullName() == seedType.FullName()).FirstOrDefault(); + if (contractType == null) + throw new Exception("Type was not found in the contract."); + + // Recursively add forwarders for all the nested types specified in the contract assembly. // NOTE: Some design-time tools can resolve forwarded nested types with only the top-level forwarder, // but the runtime currently throws a TypeLoadException without explicit forwarders for the nested // types. - foreach (var nestedType in seedType.NestedTypes.OrderBy(t => t.Name.Value)) - if (nestedType.IsVisibleOutsideAssembly()) - AddTypeForward(assembly, nestedType); + foreach (var nestedType in contractType.NestedTypes.OrderBy(t => t.Name.Value)) + { + var seedNestedType = seedType.NestedTypes.Where(nt => nt.Name.Value == nestedType.Name.Value).FirstOrDefault(); + if (seedNestedType == null) + throw new Exception("Nested type found in the contract, but not on the seedType."); + AddTypeForward(assembly, seedNestedType, contractAssembly); + } } private void AddWin32VersionResource(string contractLocation, Assembly facade) From fe9b4f104102a534c16b6833f14abf80643c7be5 Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Tue, 22 Sep 2015 17:55:48 -0700 Subject: [PATCH 4/6] Adding special case for ExportedTypes, since they should be copied with all of their nested types always --- src/GenFacades/Program.cs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index 36cfb81a00..c445e9e6a4 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -485,21 +485,24 @@ private void AddTypeForward(Assembly assembly, INamedTypeDefinition seedType, IA assembly.ExportedTypes = new List(); assembly.ExportedTypes.Add(alias); - INamedTypeDefinition contractType = contractAssembly.GetAllTypes().Where(t => t.FullName() == seedType.FullName()).FirstOrDefault(); - if (contractType == null) - throw new Exception("Type was not found in the contract."); - - // Recursively add forwarders for all the nested types specified in the contract assembly. + // Recursively add forwarders for nested types. // NOTE: Some design-time tools can resolve forwarded nested types with only the top-level forwarder, // but the runtime currently throws a TypeLoadException without explicit forwarders for the nested // types. - foreach (var nestedType in contractType.NestedTypes.OrderBy(t => t.Name.Value)) + INamedTypeDefinition contractType = contractAssembly.GetAllTypes().Where(t => t.FullName() == seedType.FullName()).FirstOrDefault(); + if (contractType != null) // Only add forwarders for the nested types on the contract { - var seedNestedType = seedType.NestedTypes.Where(nt => nt.Name.Value == nestedType.Name.Value).FirstOrDefault(); - if (seedNestedType == null) - throw new Exception("Nested type found in the contract, but not on the seedType."); - AddTypeForward(assembly, seedNestedType, contractAssembly); + foreach (var nestedType in contractType.NestedTypes.OrderBy(t => t.Name.Value)) + { + var seedNestedType = seedType.NestedTypes.Where(nt => nt.Name.Value == nestedType.Name.Value).FirstOrDefault(); + if (seedNestedType == null) + throw new Exception("Nested type found in the contract, but not on the seedType."); + AddTypeForward(assembly, seedNestedType, contractAssembly); + } } + else // Add all the nested types for ExportedTypes. + foreach (var nestedType in seedType.NestedTypes.OrderBy(t => t.Name.Value)) + AddTypeForward(assembly, nestedType, contractAssembly); } private void AddWin32VersionResource(string contractLocation, Assembly facade) From 208ca145eeef10c0baae42bc7a3be65ffe0ab540 Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Mon, 12 Oct 2015 11:57:34 -0700 Subject: [PATCH 5/6] Changing TypeDictionary to INamedTypeDefinition so we can also add the Nested types there and include them in the docId list as well. --- src/GenFacades/Program.cs | 102 ++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 42 deletions(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index c445e9e6a4..90a48044ee 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -312,33 +312,62 @@ private static IEnumerable EnumerateDocIdsToForward(IAssembly contractAs var typesToForward = contractAssembly.GetAllTypes().Where(t => TypeHelper.IsVisibleOutsideAssembly(t)) .OfType(); + List result = typeForwardsToForward.Concat(typesToForward) + .Select(type => TypeHelper.GetTypeName(type, NameFormattingOptions.DocumentationId)).ToList(); + + foreach(var type in typesToForward) + { + AddNestedTypeDocIds(result, type); + } + + return result; + } - return typeForwardsToForward.Concat(typesToForward) - .Select(type => TypeHelper.GetTypeName(type, NameFormattingOptions.DocumentationId)); + private static void AddNestedTypeDocIds(List docIds, INamedTypeDefinition type) + { + foreach (var nestedType in type.NestedTypes) + { + if (TypeHelper.IsVisibleOutsideAssembly(nestedType)) + docIds.Add(TypeHelper.GetTypeName(nestedType, NameFormattingOptions.DocumentationId)); + AddNestedTypeDocIds(docIds, nestedType); + } } - private static IReadOnlyDictionary> GenerateTypeTable(IEnumerable seedAssemblies) + private static IReadOnlyDictionary> GenerateTypeTable(IEnumerable seedAssemblies) { - var typeTable = new Dictionary>(); + var typeTable = new Dictionary>(); foreach (var assembly in seedAssemblies) { - foreach (var type in assembly.GetAllTypes().OfType()) + foreach (var type in assembly.GetAllTypes().OfType()) { if (!TypeHelper.IsVisibleOutsideAssembly(type)) continue; + AddTypeAndNestedTypesToTable(typeTable, type); + } + } + return typeTable; + } - IReadOnlyList seedTypes; - string docId = TypeHelper.GetTypeName(type, NameFormattingOptions.DocumentationId); - if (!typeTable.TryGetValue(docId, out seedTypes)) - { - seedTypes = new List(1); - typeTable.Add(docId, seedTypes); - } + private static void AddTypeAndNestedTypesToTable(Dictionary> typeTable, INamedTypeDefinition type) + { + if (type != null) + { + IReadOnlyList seedTypes; + string docId = TypeHelper.GetTypeName(type, NameFormattingOptions.DocumentationId); + if (!typeTable.TryGetValue(docId, out seedTypes)) + { + seedTypes = new List(1); + typeTable.Add(docId, seedTypes); + } + if (!seedTypes.Contains(type)) + ((List)seedTypes).Add(type); - ((List)seedTypes).Add(type); + foreach (INestedTypeDefinition nestedType in type.NestedTypes) + { + if (TypeHelper.IsVisibleOutsideAssembly(nestedType)) + AddTypeAndNestedTypesToTable(typeTable, nestedType); } } - return typeTable; } private class FacadeGenerator @@ -346,7 +375,7 @@ private class FacadeGenerator private readonly IMetadataHost _seedHost; private readonly IMetadataHost _contractHost; private readonly IReadOnlyDictionary> _docIdTable; - private readonly IReadOnlyDictionary> _typeTable; + private readonly IReadOnlyDictionary> _typeTable; private readonly IReadOnlyDictionary _seedTypePreferences; private readonly bool _clearBuildAndRevision; private readonly bool _buildDesignTimeFacades; @@ -356,7 +385,7 @@ public FacadeGenerator( IMetadataHost seedHost, IMetadataHost contractHost, IReadOnlyDictionary> docIdTable, - IReadOnlyDictionary> typeTable, + IReadOnlyDictionary> typeTable, IReadOnlyDictionary seedTypePreferences, bool clearBuildAndRevision, bool buildDesignTimeFacades, @@ -398,7 +427,7 @@ public Assembly GenerateFacade(IAssembly contractAssembly, IAssemblyReference se IEnumerable missingDocIds = docIds.Where(id => !existingDocIds.Contains(id)); foreach (string docId in missingDocIds) { - IReadOnlyList seedTypes; + IReadOnlyList seedTypes; if (!_typeTable.TryGetValue(docId, out seedTypes)) { if (!ignoreMissingTypes) @@ -409,7 +438,7 @@ public Assembly GenerateFacade(IAssembly contractAssembly, IAssemblyReference se continue; } - INamespaceTypeDefinition seedType = GetSeedType(docId, seedTypes); + INamedTypeDefinition seedType = GetSeedType(docId, seedTypes); if (seedType == null) { TraceDuplicateSeedTypeError(docId, seedTypes); @@ -447,7 +476,7 @@ public Assembly GenerateFacade(IAssembly contractAssembly, IAssemblyReference se return assembly; } - private INamespaceTypeDefinition GetSeedType(string docId, IReadOnlyList seedTypes) + private INamedTypeDefinition GetSeedType(string docId, IReadOnlyList seedTypes) { Debug.Assert(seedTypes.Count != 0); // we should already have checked for non-existent types. @@ -459,22 +488,30 @@ private INamespaceTypeDefinition GetSeedType(string docId, IReadOnlyList String.Equals(t.ContainingUnitNamespace.Unit.Name.Value, preferredSeedAssembly, StringComparison.OrdinalIgnoreCase)); + return seedTypes.SingleOrDefault(t => String.Equals(GetContainingUnitNamespaceFromType(t), preferredSeedAssembly, StringComparison.OrdinalIgnoreCase)); } return null; } - private static void TraceDuplicateSeedTypeError(string docId, IReadOnlyList seedTypes) + private static void TraceDuplicateSeedTypeError(string docId, IReadOnlyList seedTypes) { Trace.TraceError("The type '{0}' is defined in multiple seed assemblies. If this is intentional, specify one of the following arguments to choose the preferred seed type:", docId); - foreach (INamespaceTypeDefinition type in seedTypes) + foreach (INamedTypeDefinition type in seedTypes) { - Trace.TraceError(" /preferSeedType:{0}={1}", docId.Substring("T:".Length), type.ContainingUnitNamespace.Unit.Name.Value); + Trace.TraceError(" /preferSeedType:{0}={1}", docId.Substring("T:".Length), GetContainingUnitNamespaceFromType(type)); } } + private static string GetContainingUnitNamespaceFromType(INamedTypeDefinition type) + { + INamespaceTypeDefinition namespaceTypeDefinition = type as INamespaceTypeDefinition; + if (namespaceTypeDefinition != null) + return namespaceTypeDefinition.ContainingUnitNamespace.Unit.Name.Value; + return type.GetAssembly().Name.Value; + } + private void AddTypeForward(Assembly assembly, INamedTypeDefinition seedType, IAssembly contractAssembly) { var alias = new NamespaceAliasForType(); @@ -484,25 +521,6 @@ private void AddTypeForward(Assembly assembly, INamedTypeDefinition seedType, IA if (assembly.ExportedTypes == null) assembly.ExportedTypes = new List(); assembly.ExportedTypes.Add(alias); - - // Recursively add forwarders for nested types. - // NOTE: Some design-time tools can resolve forwarded nested types with only the top-level forwarder, - // but the runtime currently throws a TypeLoadException without explicit forwarders for the nested - // types. - INamedTypeDefinition contractType = contractAssembly.GetAllTypes().Where(t => t.FullName() == seedType.FullName()).FirstOrDefault(); - if (contractType != null) // Only add forwarders for the nested types on the contract - { - foreach (var nestedType in contractType.NestedTypes.OrderBy(t => t.Name.Value)) - { - var seedNestedType = seedType.NestedTypes.Where(nt => nt.Name.Value == nestedType.Name.Value).FirstOrDefault(); - if (seedNestedType == null) - throw new Exception("Nested type found in the contract, but not on the seedType."); - AddTypeForward(assembly, seedNestedType, contractAssembly); - } - } - else // Add all the nested types for ExportedTypes. - foreach (var nestedType in seedType.NestedTypes.OrderBy(t => t.Name.Value)) - AddTypeForward(assembly, nestedType, contractAssembly); } private void AddWin32VersionResource(string contractLocation, Assembly facade) From 7080834907118c8c4dce958a56fa8ee9f9d72b31 Mon Sep 17 00:00:00 2001 From: Jose Perez Rodriguez Date: Tue, 13 Oct 2015 11:05:26 -0700 Subject: [PATCH 6/6] PR feedback --- src/GenFacades/Program.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/GenFacades/Program.cs b/src/GenFacades/Program.cs index 90a48044ee..0fbf18db11 100644 --- a/src/GenFacades/Program.cs +++ b/src/GenFacades/Program.cs @@ -446,7 +446,7 @@ public Assembly GenerateFacade(IAssembly contractAssembly, IAssemblyReference se continue; } - AddTypeForward(assembly, seedType, contractAssembly); + AddTypeForward(assembly, seedType); } if (error) @@ -488,7 +488,7 @@ private INamedTypeDefinition GetSeedType(string docId, IReadOnlyList String.Equals(GetContainingUnitNamespaceFromType(t), preferredSeedAssembly, StringComparison.OrdinalIgnoreCase)); + return seedTypes.SingleOrDefault(t => String.Equals(t.GetAssembly().Name.Value, preferredSeedAssembly, StringComparison.OrdinalIgnoreCase)); } return null; @@ -500,19 +500,11 @@ private static void TraceDuplicateSeedTypeError(string docId, IReadOnlyList