From 5fbd0da6c7362117c4d7980390a63a6711717a56 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 26 May 2022 15:40:58 -0700 Subject: [PATCH 1/4] Support byref fields in GenAPI Support for ref fields is being added in .NET 7 / C# 11.0. This will impact reference assembly generation as ref fields impact how C# consumes types that contain them. A ref field represents a change to the metadata format and that can cause issues with tool chains that are not updated to understand this metadata change. A concrete example is C++/CLI which will likely error if it consumes a ref field. The following invariants must be preserved when not exposing ref fields: - The containing type can never be considered unmanaged - The type of the field, sans ref, is still important for generic expansion calculations --- .../Extensions/CSharp/CSharpCciExtensions.cs | 12 +++++++++--- .../Extensions/TypeExtensions.cs | 3 ++- .../Writers/CSharp/CSharpWriter.cs | 6 ++++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Cci.Extensions/Extensions/CSharp/CSharpCciExtensions.cs b/src/Microsoft.Cci.Extensions/Extensions/CSharp/CSharpCciExtensions.cs index 4f9c86e6045..44461e0f7c7 100644 --- a/src/Microsoft.Cci.Extensions/Extensions/CSharp/CSharpCciExtensions.cs +++ b/src/Microsoft.Cci.Extensions/Extensions/CSharp/CSharpCciExtensions.cs @@ -155,8 +155,7 @@ public static bool IsOrContainsReferenceType(this ITypeReference type) if (resolvedType.IsReferenceType) return true; - // ByReference is a special type understood by runtime to hold a ref T. - if (resolvedType.AreGenericTypeEquivalent(ByReferenceFullName)) + if (resolvedType.IsByRef()) return true; foreach (var field in resolvedType.Fields.Where(f => !f.IsStatic)) @@ -189,7 +188,7 @@ public static bool IsOrContainsNonEmptyStruct(this ITypeReference type) if (typeToCheck.TypeCode != PrimitiveTypeCode.NotPrimitive && typeToCheck.TypeCode != PrimitiveTypeCode.Invalid) return true; - if (resolvedType is Dummy || resolvedType.IsReferenceType || resolvedType.AreGenericTypeEquivalent(ByReferenceFullName)) + if (resolvedType is Dummy || resolvedType.IsReferenceType || resolvedType.IsByRef()) { if (node == 0) { @@ -394,6 +393,13 @@ public static bool IsUnsafeType(this ITypeReference type) return type.TypeCode == PrimitiveTypeCode.Pointer; } + public static bool IsByRef(this ITypeReference type) + { + // ByReference is a special type understood by runtime to hold a ref T. + // ByReference was removed in .NET 7 since support for ref T in C# 11 was introduced. + return type.TypeCode == PrimitiveTypeCode.Reference || type.AreGenericTypeEquivalent(ByReferenceFullName); + } + public static bool IsMethodUnsafe(this IMethodDefinition method) { foreach (var p in method.Parameters) diff --git a/src/Microsoft.Cci.Extensions/Extensions/TypeExtensions.cs b/src/Microsoft.Cci.Extensions/Extensions/TypeExtensions.cs index be42d38ad3f..1b48f016b2f 100644 --- a/src/Microsoft.Cci.Extensions/Extensions/TypeExtensions.cs +++ b/src/Microsoft.Cci.Extensions/Extensions/TypeExtensions.cs @@ -413,7 +413,8 @@ public static ITypeReference UnWrap(this ITypeReference reference) || reference is INamespaceTypeReference || reference is IGenericTypeParameterReference || reference is IGenericMethodParameterReference - || reference is IFunctionPointerTypeReference, + || reference is IFunctionPointerTypeReference + || reference is IManagedPointerType, string.Format("Unexpected type reference that we may need to unwrap {0}", (reference != null ? reference.GetType().FullName : "null"))); return reference; diff --git a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs index 5ebc9c33ec1..4a36d0247b4 100644 --- a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs +++ b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs @@ -159,7 +159,7 @@ public override void Visit(ITypeDefinition parentType, IEnumerable newFields = new List(); - var includedVisibleFields = fields.Where(f => _cciFilter.Include(f)); + + // Do not include ByRef fields as they would introduce metadata that some compiler may find troublesome (e.g., C++/CLI). + var includedVisibleFields = fields.Where(f => _cciFilter.Include(f) && !f.Type.IsByRef()); includedVisibleFields = includedVisibleFields.OrderBy(GetMemberKey, StringComparer.OrdinalIgnoreCase); var excludedFields = fields.Except(includedVisibleFields).Where(f => !f.IsStatic); From adc6b1427b2d1c21b27523c3d47b9d8b68d0e10f Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 26 May 2022 21:09:10 -0700 Subject: [PATCH 2/4] Public ref fields are should be left alone. --- src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs index 4a36d0247b4..337fad3bdf4 100644 --- a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs +++ b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs @@ -184,9 +184,7 @@ public override void Visit(ITypeDefinition parentType, IEnumerable newFields = new List(); - - // Do not include ByRef fields as they would introduce metadata that some compiler may find troublesome (e.g., C++/CLI). - var includedVisibleFields = fields.Where(f => _cciFilter.Include(f) && !f.Type.IsByRef()); + var includedVisibleFields = fields.Where(f => _cciFilter.Include(f)); includedVisibleFields = includedVisibleFields.OrderBy(GetMemberKey, StringComparer.OrdinalIgnoreCase); var excludedFields = fields.Except(includedVisibleFields).Where(f => !f.IsStatic); From 5b15d9e844f9aa34e46f795f0389e490966f0298 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 26 May 2022 22:03:45 -0700 Subject: [PATCH 3/4] Add the ref keyword to ByRef fields. --- .../Writers/CSharp/CSDeclarationWriter.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs index eee1bc96170..79255c6ecfd 100644 --- a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs +++ b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs @@ -437,6 +437,11 @@ void WriteTypeNameInner(ITypeReference typeReference) WriteSymbol("]"); } + else if (type.IsByRef()) + { + WriteSymbol("ref", addSpace: true); + WriteTypeNameInner(((IManagedPointerType)type).TargetType); + } else { WriteTypeNameInner(type); From 31e6684ecb3ca0207a2115f2dadf2a707e947e72 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 1 Jun 2022 12:02:35 -0700 Subject: [PATCH 4/4] Support "ref readonly" fields. --- .../CSharp/CSDeclarationWriter.Properties.cs | 2 +- .../Writers/CSharp/CSDeclarationWriter.cs | 34 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Properties.cs b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Properties.cs index 4e4cd170b5b..c68e02ba585 100644 --- a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Properties.cs +++ b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Properties.cs @@ -104,7 +104,7 @@ private void WritePropertyDefinition(IPropertyDefinition property) WriteKeyword("readonly"); } - WriteTypeName(property.Type, attributes: property.Attributes); + WriteTypeName(property.Type, property.Attributes); if (property.IsExplicitInterfaceProperty() && _forCompilationIncludeGlobalprefix) Write("global::"); diff --git a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs index 79255c6ecfd..53fbe66caf4 100644 --- a/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs +++ b/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs @@ -6,7 +6,6 @@ using Microsoft.Cci.Filters; using Microsoft.Cci.Writers.Syntax; using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Linq; @@ -282,13 +281,20 @@ private bool IsDynamicType(object dynamicAttributeArgument, int arrayIndex) } return (bool)dynamicAttributeArgument; + } + private ref struct TypeNameRecursiveState + { + public object DynamicAttributeArgument; + public object NullableAttributeArgument; + public IEnumerable Attributes; } private int WriteTypeNameRecursive(ITypeReference type, NameFormattingOptions namingOptions, - string[] valueTupleNames, ref int valueTupleNameIndex, ref int nullableIndex, object nullableAttributeArgument, object dynamicAttributeArgument, + string[] valueTupleNames, ref int valueTupleNameIndex, ref int nullableIndex, ref TypeNameRecursiveState state, int typeDepth = 0, int genericParameterIndex = 0, bool isValueTupleParameter = false) { + object dynamicAttributeArgument = state.DynamicAttributeArgument; void WriteTypeNameInner(ITypeReference typeReference) { if (IsDynamicType(dynamicAttributeArgument, typeDepth)) @@ -386,7 +392,7 @@ void WriteTypeNameInner(ITypeReference typeReference) string valueTupleName = isValueTuple ? valueTupleNames?[valueTupleLocalIndex + i] : null; int destinationTypeDepth = typeDepth + i + genericArgumentsInChildTypes + 1; - genericArgumentsInChildTypes += WriteTypeNameRecursive(parameter, namingOptions, valueTupleNames, ref valueTupleNameIndex, ref nullableIndex, nullableAttributeArgument, dynamicAttributeArgument, destinationTypeDepth, i, isValueTuple); + genericArgumentsInChildTypes += WriteTypeNameRecursive(parameter, namingOptions, valueTupleNames, ref valueTupleNameIndex, ref nullableIndex, ref state, destinationTypeDepth, i, isValueTuple); if (valueTupleName != null) { @@ -426,7 +432,7 @@ void WriteTypeNameInner(ITypeReference typeReference) nullableIndex++; WriteTypeNameRecursive(arrayType.ElementType, namingOptions, valueTupleNames, ref valueTupleNameIndex, ref nullableIndex, - nullableAttributeArgument, dynamicAttributeArgument, typeDepth + 1); + ref state, typeDepth + 1); WriteSymbol("["); uint arrayDimension = arrayType.Rank - 1; @@ -440,6 +446,10 @@ void WriteTypeNameInner(ITypeReference typeReference) else if (type.IsByRef()) { WriteSymbol("ref", addSpace: true); + + if (state.Attributes.HasIsReadOnlyAttribute()) + WriteSymbol("readonly", addSpace: true); + WriteTypeNameInner(((IManagedPointerType)type).TargetType); } else @@ -453,7 +463,7 @@ void WriteTypeNameInner(ITypeReference typeReference) } else if (!type.IsValueType) { - WriteNullableSymbolForReferenceType(nullableAttributeArgument, nullableLocalIndex); + WriteNullableSymbolForReferenceType(state.NullableAttributeArgument, nullableLocalIndex); } return genericArgumentsCount; @@ -471,13 +481,13 @@ private void WriteTypeName(ITypeReference type, IEnumerable at object dynamicAttributeArgument = dynamicAttribute.GetAttributeArgumentValue(defaultValue: hasDynamicAttribute); - WriteTypeName(type, noSpace, useTypeKeywords, omitGenericTypeList, nullableAttributeArgument, dynamicAttributeArgument, attributes?.GetValueTupleNames()); + WriteTypeName(type, noSpace, useTypeKeywords, omitGenericTypeList, nullableAttributeArgument, dynamicAttributeArgument, attributes); } private void WriteTypeName(ITypeReference type, bool noSpace = false, bool useTypeKeywords = true, - bool omitGenericTypeList = false, object nullableAttributeArgument = null, object dynamicAttributeArgument = null, string[] valueTupleNames = null) + bool omitGenericTypeList = false, object nullableAttributeArgument = null, object dynamicAttributeArgument = null, IEnumerable attributes = null) { - NameFormattingOptions namingOptions = NameFormattingOptions.TypeParameters | NameFormattingOptions.ContractNullable | NameFormattingOptions.OmitTypeArguments; ; + NameFormattingOptions namingOptions = NameFormattingOptions.TypeParameters | NameFormattingOptions.ContractNullable | NameFormattingOptions.OmitTypeArguments; if (useTypeKeywords) namingOptions |= NameFormattingOptions.UseTypeKeywords; @@ -493,7 +503,13 @@ private void WriteTypeName(ITypeReference type, bool noSpace = false, bool useTy int valueTupleNameIndex = 0; int nullableIndex = 0; - WriteTypeNameRecursive(type, namingOptions, valueTupleNames, ref valueTupleNameIndex, ref nullableIndex, nullableAttributeArgument, dynamicAttributeArgument); + var state = new TypeNameRecursiveState() + { + DynamicAttributeArgument = dynamicAttributeArgument, + NullableAttributeArgument = nullableAttributeArgument, + Attributes = attributes, + }; + WriteTypeNameRecursive(type, namingOptions, attributes?.GetValueTupleNames(), ref valueTupleNameIndex, ref nullableIndex, ref state); if (!noSpace) WriteSpace(); }