From 86595de9f6dbc8a17e87b4581b38e0341af12bc4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 18 Jun 2024 16:24:30 +0200 Subject: [PATCH 01/18] new TypeName methods --- .../Formats/Nrbf/ArraySingleObjectRecord.cs | 2 +- .../Nrbf/ArraySinglePrimitiveRecord.cs | 2 +- .../Formats/Nrbf/ArraySingleStringRecord.cs | 2 +- ...peNameExtensions.cs => TypeNameHelpers.cs} | 49 ++----- .../ref/System.Reflection.Metadata.cs | 6 + .../System/Reflection/Metadata/TypeName.cs | 122 ++++++++++++++++++ .../tests/Metadata/TypeNameTests.cs | 66 ++++++++-- 7 files changed, 198 insertions(+), 51 deletions(-) rename src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/{TypeNameExtensions.cs => TypeNameHelpers.cs} (58%) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs index de77d3e0f84fac..d692c03905e3d0 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs @@ -23,7 +23,7 @@ internal sealed class ArraySingleObjectRecord : SZArrayRecord public override SerializationRecordType RecordType => SerializationRecordType.ArraySingleObject; public override TypeName TypeName - => s_typeName ??= TypeName.Parse(("System.Object[], " + TypeNameExtensions.CoreLibAssemblyName).AsSpan()); + => s_typeName ??= TypeName.Parse("System.Object[]".AsSpan()).WithCoreLibAssemblyName(); private List Records { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs index a09ca6b7e48d27..b2a0bf21ec1aa7 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs @@ -36,7 +36,7 @@ internal ArraySinglePrimitiveRecord(ArrayInfo arrayInfo, IReadOnlyList values /// public override TypeName TypeName - => s_typeName ??= TypeName.Parse((typeof(T[]).FullName + "," + TypeNameExtensions.CoreLibAssemblyName).AsSpan()); + => s_typeName ??= TypeName.Parse(typeof(T[]).FullName.AsSpan()).WithAssemblyName(TypeNameHelpers.s_CoreLibAssemblyName); internal IReadOnlyList Values { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs index 4d7f5ace476791..9d3a4de92c042f 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs @@ -24,7 +24,7 @@ internal sealed class ArraySingleStringRecord : SZArrayRecord /// public override TypeName TypeName - => s_typeName ??= TypeName.Parse(("System.String[], " + TypeNameExtensions.CoreLibAssemblyName).AsSpan()); + => s_typeName ??= TypeName.Parse("System.String[]".AsSpan()).WithCoreLibAssemblyName(); private List Records { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameExtensions.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs similarity index 58% rename from src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameExtensions.cs rename to src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index 017f4597bfc2de..05cda439343248 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameExtensions.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -10,9 +10,9 @@ namespace System.Formats.Nrbf.Utils; -internal static class TypeNameExtensions +internal static class TypeNameHelpers { - internal const string CoreLibAssemblyName = "mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"; + internal static readonly AssemblyNameInfo s_CoreLibAssemblyName = AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan()); internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, BinaryLibraryRecord libraryRecord, PayloadOptions payloadOptions) { @@ -42,49 +42,26 @@ internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadOptions payloadOptions) { - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(rawName, - CoreLibAssemblyName); // We know it's a System Record, so we set the LibraryName to CoreLib - - TypeName.TryParse(assemblyQualifiedName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions); - ArrayPool.Shared.Return(assemblyQualifiedName.Array!); + if (!TypeName.TryParse(rawName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions)) + { + throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeName, rawName)); + } - return typeName ?? throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeName, rawName)); + // We know it's a System Record, so we set the LibraryName to CoreLib + return WithCoreLibAssemblyName(typeName); } internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(CoreLibAssemblyName); - - internal static TypeName WithAssemblyName(this TypeName typeName, string assemblyName) - { - // For ClassWithMembersAndTypesRecord, the TypeName and LibraryName and provided separately, - // and the LibraryName may not be known when parsing TypeName. - // For SystemClassWithMembersAndTypesRecord, the LibraryName is not provided, it's always mscorlib. - // Ideally, we would just create TypeName with new AssemblyNameInfo. - // This will be possible once https://github.com/dotnet/runtime/issues/102263 is done. - - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(typeName.FullName, assemblyName); - TypeName result = TypeName.Parse(assemblyQualifiedName.AsSpan()); - ArrayPool.Shared.Return(assemblyQualifiedName.Array!); - - return result; - } + => systemType.WithAssemblyName(s_CoreLibAssemblyName); internal static TypeName BuildCoreLibArrayTypeName(this Type type, int arrayRank) - { - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(type.FullName!, CoreLibAssemblyName, arrayRank); - TypeName result = TypeName.Parse(assemblyQualifiedName.AsSpan()); - ArrayPool.Shared.Return(assemblyQualifiedName.Array!); - - return result; - } + => WithCoreLibAssemblyName(BuildArrayTypeName(TypeName.Parse(type.FullName.AsSpan()), arrayRank)); internal static TypeName BuildArrayTypeName(this TypeName typeName, int arrayRank) { - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(typeName.FullName, typeName.AssemblyName!.FullName, arrayRank); - TypeName result = TypeName.Parse(assemblyQualifiedName.AsSpan()); - ArrayPool.Shared.Return(assemblyQualifiedName.Array!); - - return result; + // In this particular context, arrayRank == 1 means SZArray (custom offset arrays are not supported by design). + // That is why we don't call typeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. + return arrayRank == 1 ? typeName.MakeArrayTypeName() : typeName.MakeArrayTypeName(arrayRank); } private static ArraySegment GetAssemblyQualifiedName(string typeName, string libraryName, int arrayRank = 0) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index 6eaf4ea70e61ee..b01b1e964591a4 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2445,6 +2445,12 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName GetGenericTypeDefinition() { throw null; } public System.Reflection.Metadata.TypeName GetElementType() { throw null; } public int GetNodeCount() { throw null; } + public System.Reflection.Metadata.TypeName MakeArrayTypeName() { throw null; } + public System.Reflection.Metadata.TypeName MakeArrayTypeName(int rank) { throw null; } + public System.Reflection.Metadata.TypeName MakeByRefTypeName() { throw null; } + public System.Reflection.Metadata.TypeName MakeGenericTypeName(System.Collections.Immutable.ImmutableArray typeArguments) { throw null; } + public System.Reflection.Metadata.TypeName MakePointerTypeName() { throw null; } + public System.Reflection.Metadata.TypeName WithAssemblyName(AssemblyNameInfo assemblyName) { throw null; } } public sealed partial class TypeNameParseOptions { diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 3f1ea8394393c8..27f460656ca697 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -69,6 +69,25 @@ internal TypeName(string? fullName, #endif } +#if SYSTEM_REFLECTION_METADATA + private TypeName(string? fullName, + AssemblyNameInfo? assemblyName, + TypeName? elementOrGenericType = default, + TypeName? declaringType = default, + ImmutableArray genericTypeArguments = default, + sbyte rankOrModifier = default, + int nestedNameLength = -1) + { + _fullName = fullName; + AssemblyName = assemblyName; + _elementOrGenericType = elementOrGenericType; + _declaringType = declaringType; + _genericArguments = genericTypeArguments; + _rankOrModifier = rankOrModifier; + _nestedNameLength = nestedNameLength; + } +#endif + /// /// The assembly-qualified name of the type; e.g., "System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089". /// @@ -392,5 +411,108 @@ public int GetArrayRank() #else ImmutableArray GetGenericArguments() => _genericArguments; #endif + +#if SYSTEM_REFLECTION_METADATA + /// + /// Returns a object representing a one-dimensional array + /// of the current type, with a lower bound of zero. + /// + /// + /// A object representing a one-dimensional array + /// of the current type, with a lower bound of zero. + /// + public TypeName MakeArrayTypeName() => MakeElementTypeName(TypeNameParserHelpers.SZArray); + + /// + /// Returns a object representing an array of the current type, + /// with the specified number of dimensions. + /// + /// + /// A object representing an array of the current type, + /// with the specified number of dimensions. + /// + /// rank is invalid. For example, 0 or negative. + public TypeName MakeArrayTypeName(int rank) + => rank <= 0 || rank > 32 + ? throw new ArgumentOutOfRangeException(nameof(rank)) + : MakeElementTypeName((sbyte)rank); + + /// + /// Returns a object that represents a pointer to the current type. + /// + /// + /// A object that represents a pointer to the current type. + /// + public TypeName MakePointerTypeName() => MakeElementTypeName(TypeNameParserHelpers.Pointer); + + /// + /// Returns a object that represents a managed reference to the current type. + /// + /// + /// A object that represents a managed reference to the current type. + /// + public TypeName MakeByRefTypeName() => MakeElementTypeName(TypeNameParserHelpers.ByRef); + + public TypeName MakeGenericTypeName(ImmutableArray typeArguments) + => new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, genericTypeArguments: typeArguments); + + // Nulls are allowed, as they allow for simply removing the name. + public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) + => WithAssemblyName(this, AssemblyName, assemblyName)!; + + private static TypeName? WithAssemblyName(TypeName? typeName, AssemblyNameInfo? oldName, AssemblyNameInfo? newName) + { + if (typeName is null) + { + return null; + } + else if (oldName is not null && !ReferenceEquals(typeName.AssemblyName, oldName)) + { + // There is nothing to update, do nothing. + return typeName; + } + + ImmutableArray genericArguments = typeName._genericArguments; + + if (typeName.IsConstructedGenericType) + { + bool needsNewArray = false; + for (int i = 0; i < typeName._genericArguments.Length; i++) + { + if (ReferenceEquals(typeName._genericArguments[i].AssemblyName, oldName)) + { + needsNewArray = true; + } + } + + if (needsNewArray) + { + ImmutableArray.Builder builder = ImmutableArray.CreateBuilder(typeName._genericArguments.Length); + for (int i = 0; i < typeName._genericArguments.Length; i++) + { + builder.Add(WithAssemblyName(typeName._genericArguments[i], oldName, newName)!); + } + genericArguments = builder.MoveToImmutable(); + } + } + + return new TypeName( + fullName: typeName._fullName, + newName, + elementOrGenericType: WithAssemblyName(typeName._elementOrGenericType, oldName, newName), + declaringType: WithAssemblyName(typeName._declaringType, oldName, newName), + genericTypeArguments: genericArguments, + rankOrModifier: typeName._rankOrModifier, + nestedNameLength: typeName._nestedNameLength); + } + + private TypeName MakeElementTypeName(sbyte rankOrModifier) + => new TypeName( + fullName: null, + assemblyName: AssemblyName, + elementOrGenericType: this, + genericTypeArguments: ImmutableArray.Empty, + rankOrModifier: rankOrModifier); +#endif } } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index a5e22b5d6c503b..df21197ef065f5 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -107,9 +107,7 @@ public void TypeNameCanContainAssemblyName(Type type) static void Verify(Type type, AssemblyName expectedAssemblyName, TypeName parsed) { - Assert.Equal(type.AssemblyQualifiedName, parsed.AssemblyQualifiedName); - Assert.Equal(type.FullName, parsed.FullName); - Assert.Equal(type.Name, parsed.Name); + EnsureBasicMatch(parsed, type); AssemblyNameInfo parsedAssemblyName = parsed.AssemblyName; Assert.NotNull(parsedAssemblyName); @@ -484,9 +482,41 @@ public void GetNodeCountReturnsExpectedValue(Type type, int expected) Assert.Equal(expected, parsed.GetNodeCount()); - Assert.Equal(type.Name, parsed.Name); - Assert.Equal(type.FullName, parsed.FullName); - Assert.Equal(type.AssemblyQualifiedName, parsed.AssemblyQualifiedName); + EnsureBasicMatch(parsed, type); + + if (!type.IsByRef) + { + EnsureBasicMatch(parsed.MakeArrayTypeName(), type.MakeArrayType()); + EnsureBasicMatch(parsed.MakeArrayTypeName(3), type.MakeArrayType(3)); + EnsureBasicMatch(parsed.MakeByRefTypeName(), type.MakeByRefType()); + EnsureBasicMatch(parsed.MakePointerTypeName(), type.MakePointerType()); + } + } + + [Fact] + public void MakeGenericTypeName() + { + Type genericTypeDefinition = typeof(Dictionary<,>); + TypeName genericTypeNameDefinition = TypeName.Parse(genericTypeDefinition.AssemblyQualifiedName.AsSpan()); + + Type[] genericArgs = new Type[] { typeof(int), typeof(bool) }; + EnsureBasicMatch( + genericTypeNameDefinition.MakeGenericTypeName(genericArgs.Select(type => TypeName.Parse(type.AssemblyQualifiedName.AsSpan())).ToImmutableArray()), + genericTypeDefinition.MakeGenericType(genericArgs)); + } + + [Theory] + [InlineData("Namespace.Simple")] + [InlineData("Namespace.Simple, SomeAssembly")] + public void WithAssemblyName_SimpleNames(string name) + { + TypeName simpleName = TypeName.Parse(name.AsSpan()); + Assert.Equal(name, simpleName.AssemblyQualifiedName); + AssemblyNameInfo assemblyName = new("Different"); + + TypeName withDifferentAssemblyName = simpleName.WithAssemblyName(assemblyName); + Assert.Same(assemblyName, withDifferentAssemblyName.AssemblyName); + Assert.Equal("Namespace.Simple, Different", withDifferentAssemblyName.AssemblyQualifiedName); } [Fact] @@ -561,9 +591,7 @@ public void ParsedNamesMatchSystemTypeNames(Type type) { TypeName parsed = TypeName.Parse(type.AssemblyQualifiedName.AsSpan()); - Assert.Equal(type.Name, parsed.Name); - Assert.Equal(type.FullName, parsed.FullName); - Assert.Equal(type.AssemblyQualifiedName, parsed.AssemblyQualifiedName); + EnsureBasicMatch(parsed, type); if (type.IsGenericType) { @@ -626,9 +654,7 @@ static void Test(Type type) TypeName parsed = TypeName.Parse(type.AssemblyQualifiedName.AsSpan()); // ensure that Name, FullName and AssemblyQualifiedName match reflection APIs!! - Assert.Equal(type.Name, parsed.Name); - Assert.Equal(type.FullName, parsed.FullName); - Assert.Equal(type.AssemblyQualifiedName, parsed.AssemblyQualifiedName); + EnsureBasicMatch(parsed, type); // now load load the type from name Verify(type, parsed, ignoreCase: false); #if NET // something weird is going on here @@ -729,6 +755,22 @@ static void Verify(Type type, TypeName typeName, bool ignoreCase) #pragma warning restore IL2055, IL2057, IL2075, IL2096 } + private static void EnsureBasicMatch(TypeName typeName, Type type) + { + Assert.Equal(type.AssemblyQualifiedName, typeName.AssemblyQualifiedName); + Assert.Equal(type.FullName, typeName.FullName); + Assert.Equal(type.Name, typeName.Name); + +#if NET + Assert.Equal(type.IsSZArray, typeName.IsSZArray); +#endif + Assert.Equal(type.IsArray, typeName.IsArray); + Assert.Equal(type.IsPointer, typeName.IsPointer); + Assert.Equal(type.IsByRef, typeName.IsByRef); + Assert.Equal(type.IsConstructedGenericType, typeName.IsConstructedGenericType); + Assert.Equal(type.IsNested, typeName.IsNested); + } + public class NestedNonGeneric_0 { public class NestedNonGeneric_1 { } From d9066069b66a1764c4921bb1fc1a2a9225b984a4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 19 Jun 2024 09:22:16 +0200 Subject: [PATCH 02/18] do not join the type and library name strings when UndoTruncatedTypeNames is set to false, parse assembly name once and just reuse it --- .../src/Resources/Strings.resx | 3 + .../Nrbf/ArraySinglePrimitiveRecord.cs | 2 +- .../Formats/Nrbf/BinaryLibraryRecord.cs | 32 +++++++-- .../src/System/Formats/Nrbf/NrbfDecoder.cs | 2 +- .../System/Formats/Nrbf/Utils/ThrowHelper.cs | 3 + .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 71 ++++++++++--------- 6 files changed, 73 insertions(+), 40 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx b/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx index 22ce2808f24062..349405150c65e9 100644 --- a/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx +++ b/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx @@ -159,4 +159,7 @@ Only arrays with zero offsets are supported. + + Invalid assembly name: `{0}`. + \ No newline at end of file diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs index b2a0bf21ec1aa7..49cac6ca8f4612 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs @@ -36,7 +36,7 @@ internal ArraySinglePrimitiveRecord(ArrayInfo arrayInfo, IReadOnlyList values /// public override TypeName TypeName - => s_typeName ??= TypeName.Parse(typeof(T[]).FullName.AsSpan()).WithAssemblyName(TypeNameHelpers.s_CoreLibAssemblyName); + => s_typeName ??= TypeName.Parse(typeof(T[]).FullName.AsSpan()).WithCoreLibAssemblyName(); internal IReadOnlyList Values { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs index d7a92293fdd2bc..331d633c63263e 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Formats.Nrbf.Utils; using System.IO; using System.Reflection.Metadata; +using System.Runtime.Serialization; namespace System.Formats.Nrbf; @@ -15,7 +17,13 @@ namespace System.Formats.Nrbf; /// internal sealed class BinaryLibraryRecord : SerializationRecord { - private BinaryLibraryRecord(SerializationRecordId libraryId, string libraryName) + private BinaryLibraryRecord(SerializationRecordId libraryId, string rawLibraryName) + { + Id = libraryId; + RawLibraryName = rawLibraryName; + } + + private BinaryLibraryRecord(SerializationRecordId libraryId, AssemblyNameInfo libraryName) { Id = libraryId; LibraryName = libraryName; @@ -32,11 +40,27 @@ public override TypeName TypeName } } - internal string LibraryName { get; } + internal string? RawLibraryName { get; } + + internal AssemblyNameInfo? LibraryName { get; } /// public override SerializationRecordId Id { get; } - internal static BinaryLibraryRecord Decode(BinaryReader reader) - => new(SerializationRecordId.Decode(reader), reader.ReadString()); + internal static BinaryLibraryRecord Decode(BinaryReader reader, PayloadOptions options) + { + SerializationRecordId id = SerializationRecordId.Decode(reader); + string rawName = reader.ReadString(); + + if (AssemblyNameInfo.TryParse(rawName.AsSpan(), out AssemblyNameInfo? assemblyNameInfo)) + { + return new BinaryLibraryRecord(id, assemblyNameInfo); + } + else if (!options.UndoTruncatedTypeNames) + { + ThrowHelper.ThrowInvalidAssemblyName(rawName); + } + + return new BinaryLibraryRecord(id, rawName); + } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index 426755ee7fe87e..407dc98bcd8ad1 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -227,7 +227,7 @@ private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap rec SerializationRecordType.ArraySinglePrimitive => DecodeArraySinglePrimitiveRecord(reader), SerializationRecordType.ArraySingleString => ArraySingleStringRecord.Decode(reader), SerializationRecordType.BinaryArray => BinaryArrayRecord.Decode(reader, recordMap, options), - SerializationRecordType.BinaryLibrary => BinaryLibraryRecord.Decode(reader), + SerializationRecordType.BinaryLibrary => BinaryLibraryRecord.Decode(reader, options), SerializationRecordType.BinaryObjectString => BinaryObjectStringRecord.Decode(reader), SerializationRecordType.ClassWithId => ClassWithIdRecord.Decode(reader, recordMap), SerializationRecordType.ClassWithMembersAndTypes => ClassWithMembersAndTypesRecord.Decode(reader, recordMap, options), diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs index de543f2e098cf4..3689138fa14b13 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs @@ -23,6 +23,9 @@ internal static void ThrowMaxArrayLength(long limit, long actual) internal static void ThrowArrayContainedNulls() => throw new SerializationException(SR.Serialization_ArrayContainedNulls); + internal static void ThrowInvalidAssemblyName(string rawName) + => throw new SerializationException(SR.Format(SR.Serialization_InvalidAssemblyName, rawName)); + internal static void ThrowEndOfStreamException() => throw new EndOfStreamException(); diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index 05cda439343248..9cabb1aa8bf2a5 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -12,23 +12,29 @@ namespace System.Formats.Nrbf.Utils; internal static class TypeNameHelpers { - internal static readonly AssemblyNameInfo s_CoreLibAssemblyName = AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan()); + private static AssemblyNameInfo? s_CoreLibAssemblyName; internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, BinaryLibraryRecord libraryRecord, PayloadOptions payloadOptions) { - // Combining type and library name has two goals: - // 1. Handle truncated generic type names that may be present in resources. - // 2. Improve perf by parsing only once. - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(rawName, libraryRecord.LibraryName); + if (libraryRecord.LibraryName is not null) + { + return ParseWithoutAssemblyName(rawName, payloadOptions).WithAssemblyName(libraryRecord.LibraryName); + } + + Debug.Assert(payloadOptions.UndoTruncatedTypeNames); + Debug.Assert(libraryRecord.RawLibraryName is not null); + + // Combining type and library allows us for handling truncated generic type names that may be present in resources. + ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(rawName, libraryRecord.RawLibraryName); TypeName.TryParse(assemblyQualifiedName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions); ArrayPool.Shared.Return(assemblyQualifiedName.Array!); - if (typeName is null || (typeName.AssemblyName is null && !payloadOptions.UndoTruncatedTypeNames)) + if (typeName is null) { - throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeOrAssemblyName, rawName, libraryRecord.LibraryName)); + throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeOrAssemblyName, rawName, libraryRecord.RawLibraryName)); } - if (typeName.AssemblyName is null && payloadOptions.UndoTruncatedTypeNames) + if (typeName.AssemblyName is null) { // Sample invalid input that could lead us here: // TypeName: System.Collections.Generic.List`1[[System.String @@ -41,48 +47,45 @@ internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, } internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadOptions payloadOptions) - { - if (!TypeName.TryParse(rawName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions)) - { - throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeName, rawName)); - } - - // We know it's a System Record, so we set the LibraryName to CoreLib - return WithCoreLibAssemblyName(typeName); - } + => ParseWithoutAssemblyName(rawName, payloadOptions) + .WithCoreLibAssemblyName(); // We know it's a System Record, so we set the LibraryName to CoreLib internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(s_CoreLibAssemblyName); + => systemType.WithAssemblyName(s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); internal static TypeName BuildCoreLibArrayTypeName(this Type type, int arrayRank) => WithCoreLibAssemblyName(BuildArrayTypeName(TypeName.Parse(type.FullName.AsSpan()), arrayRank)); internal static TypeName BuildArrayTypeName(this TypeName typeName, int arrayRank) { - // In this particular context, arrayRank == 1 means SZArray (custom offset arrays are not supported by design). - // That is why we don't call typeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. + // In general, arrayRank == 1 may have two different meanings: + // - [] is a single dimension and zero-indexed array (SZArray) + // - [*] is single dimension, custom offset array. + // Custom offset arrays are not supported by design. + // That is why we don't call TypeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. return arrayRank == 1 ? typeName.MakeArrayTypeName() : typeName.MakeArrayTypeName(arrayRank); } - private static ArraySegment GetAssemblyQualifiedName(string typeName, string libraryName, int arrayRank = 0) + private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) { - int arrayLength = arrayRank != 0 ? 2 + arrayRank - 1 : 0; - int length = typeName.Length + arrayLength + 1 + libraryName.Length; + if (!TypeName.TryParse(rawName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions) + || typeName.AssemblyName is not null) // the type and library names should be provided separately + { + throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeName, rawName)); + } + + return typeName; + } + + private static ArraySegment GetAssemblyQualifiedName(string typeName, string libraryName) + { + int length = typeName.Length + 1 + libraryName.Length; char[] rented = ArrayPool.Shared.Rent(length); typeName.AsSpan().CopyTo(rented); - if (arrayRank != 0) - { - rented[typeName.Length] = '['; - for (int i = 1; i < arrayRank; i++) - { - rented[typeName.Length + i] = ','; - } - rented[typeName.Length + arrayLength - 1] = ']'; - } - rented[typeName.Length + arrayLength] = ','; - libraryName.AsSpan().CopyTo(rented.AsSpan(typeName.Length + arrayLength + 1)); + rented[typeName.Length] = ','; + libraryName.AsSpan().CopyTo(rented.AsSpan(typeName.Length + 1)); return new ArraySegment(rented, 0, length); } From 448f01e994c275d26cb037e23d76426f5959b79d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 19 Jun 2024 11:12:24 +0200 Subject: [PATCH 03/18] cache known primitive type names (and SZArrays of these) --- .../Formats/Nrbf/ArraySingleObjectRecord.cs | 4 +- .../Nrbf/ArraySinglePrimitiveRecord.cs | 5 +- .../Formats/Nrbf/ArraySingleStringRecord.cs | 5 +- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 76 +++++------------ .../Formats/Nrbf/PrimitiveTypeRecordOfT.cs | 5 +- .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 84 ++++++++++++++++--- 6 files changed, 94 insertions(+), 85 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs index d692c03905e3d0..37e94842719a90 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleObjectRecord.cs @@ -16,14 +16,12 @@ namespace System.Formats.Nrbf; /// internal sealed class ArraySingleObjectRecord : SZArrayRecord { - private static TypeName? s_typeName; - private ArraySingleObjectRecord(ArrayInfo arrayInfo) : base(arrayInfo) => Records = []; public override SerializationRecordType RecordType => SerializationRecordType.ArraySingleObject; public override TypeName TypeName - => s_typeName ??= TypeName.Parse("System.Object[]".AsSpan()).WithCoreLibAssemblyName(); + => TypeNameHelpers.GetPrimitiveSZArrayTypeName(TypeNameHelpers.ObjectPrimitiveType); private List Records { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs index 49cac6ca8f4612..ee3a7916b069cf 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs @@ -24,8 +24,6 @@ namespace System.Formats.Nrbf; internal sealed class ArraySinglePrimitiveRecord : SZArrayRecord where T : unmanaged { - private static TypeName? s_typeName; - internal ArraySinglePrimitiveRecord(ArrayInfo arrayInfo, IReadOnlyList values) : base(arrayInfo) { Values = values; @@ -35,8 +33,7 @@ internal ArraySinglePrimitiveRecord(ArrayInfo arrayInfo, IReadOnlyList values public override SerializationRecordType RecordType => SerializationRecordType.ArraySinglePrimitive; /// - public override TypeName TypeName - => s_typeName ??= TypeName.Parse(typeof(T[]).FullName.AsSpan()).WithCoreLibAssemblyName(); + public override TypeName TypeName => TypeNameHelpers.GetPrimitiveSZArrayTypeName(TypeNameHelpers.GetPrimitiveType()); internal IReadOnlyList Values { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs index 9d3a4de92c042f..de248bcef76755 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySingleStringRecord.cs @@ -16,15 +16,12 @@ namespace System.Formats.Nrbf; /// internal sealed class ArraySingleStringRecord : SZArrayRecord { - private static TypeName? s_typeName; - private ArraySingleStringRecord(ArrayInfo arrayInfo) : base(arrayInfo) => Records = []; public override SerializationRecordType RecordType => SerializationRecordType.ArraySingleString; /// - public override TypeName TypeName - => s_typeName ??= TypeName.Parse("System.String[]".AsSpan()).WithCoreLibAssemblyName(); + public override TypeName TypeName => TypeNameHelpers.GetPrimitiveSZArrayTypeName(PrimitiveType.String); private List Records { get; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 9e6c2445877e0b..16d35b0fc9b5e6 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -142,63 +142,25 @@ internal TypeName GetArrayTypeName(ArrayInfo arrayInfo) { (BinaryType binaryType, object? additionalInfo) = Infos[0]; - switch (binaryType) + TypeName elementTypeName = binaryType switch { - case BinaryType.String: - return typeof(string).BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.StringArray: - return typeof(string[]).BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.Object: - return typeof(object).BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.ObjectArray: - return typeof(object[]).BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.Primitive: - Type primitiveType = ((PrimitiveType)additionalInfo!) switch - { - PrimitiveType.Boolean => typeof(bool), - PrimitiveType.Byte => typeof(byte), - PrimitiveType.Char => typeof(char), - PrimitiveType.Decimal => typeof(decimal), - PrimitiveType.Double => typeof(double), - PrimitiveType.Int16 => typeof(short), - PrimitiveType.Int32 => typeof(int), - PrimitiveType.Int64 => typeof(long), - PrimitiveType.SByte => typeof(sbyte), - PrimitiveType.Single => typeof(float), - PrimitiveType.TimeSpan => typeof(TimeSpan), - PrimitiveType.DateTime => typeof(DateTime), - PrimitiveType.UInt16 => typeof(ushort), - PrimitiveType.UInt32 => typeof(uint), - _ => typeof(ulong), - }; - - return primitiveType.BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.PrimitiveArray: - Type primitiveArrayType = ((PrimitiveType)additionalInfo!) switch - { - PrimitiveType.Boolean => typeof(bool[]), - PrimitiveType.Byte => typeof(byte[]), - PrimitiveType.Char => typeof(char[]), - PrimitiveType.Decimal => typeof(decimal[]), - PrimitiveType.Double => typeof(double[]), - PrimitiveType.Int16 => typeof(short[]), - PrimitiveType.Int32 => typeof(int[]), - PrimitiveType.Int64 => typeof(long[]), - PrimitiveType.SByte => typeof(sbyte[]), - PrimitiveType.Single => typeof(float[]), - PrimitiveType.TimeSpan => typeof(TimeSpan[]), - PrimitiveType.DateTime => typeof(DateTime[]), - PrimitiveType.UInt16 => typeof(ushort[]), - PrimitiveType.UInt32 => typeof(uint[]), - _ => typeof(ulong[]), - }; - - return primitiveArrayType.BuildCoreLibArrayTypeName(arrayInfo.Rank); - case BinaryType.SystemClass: - return ((TypeName)additionalInfo!).BuildArrayTypeName(arrayInfo.Rank); - default: - Debug.Assert(binaryType is BinaryType.Class, "The parsers should reject other inputs"); - return (((ClassTypeInfo)additionalInfo!).TypeName).BuildArrayTypeName(arrayInfo.Rank); - } + BinaryType.String => TypeNameHelpers.GetPrimitiveTypeName(PrimitiveType.String), + BinaryType.StringArray => TypeNameHelpers.GetPrimitiveSZArrayTypeName(PrimitiveType.String), + BinaryType.Primitive => TypeNameHelpers.GetPrimitiveTypeName((PrimitiveType)additionalInfo!), + BinaryType.PrimitiveArray => TypeNameHelpers.GetPrimitiveSZArrayTypeName((PrimitiveType)additionalInfo!), + BinaryType.Object => TypeNameHelpers.GetPrimitiveTypeName(TypeNameHelpers.ObjectPrimitiveType), + BinaryType.ObjectArray => TypeNameHelpers.GetPrimitiveSZArrayTypeName(TypeNameHelpers.ObjectPrimitiveType), + BinaryType.SystemClass => (TypeName)additionalInfo!, + _ => ((ClassTypeInfo)additionalInfo!).TypeName, + }; + + // In general, arrayRank == 1 may have two different meanings: + // - [] is a single dimension and zero-indexed array (SZArray) + // - [*] is single dimension, custom offset array. + // Custom offset arrays are not supported by design, so in our case it's always SZArray. + // That is why we don't call TypeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. + return arrayInfo.Rank == 1 + ? elementTypeName.MakeArrayTypeName() + : elementTypeName.MakeArrayTypeName(arrayInfo.Rank); } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/PrimitiveTypeRecordOfT.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/PrimitiveTypeRecordOfT.cs index 72ef801e61eb32..80f5dc54ef1de8 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/PrimitiveTypeRecordOfT.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/PrimitiveTypeRecordOfT.cs @@ -25,8 +25,6 @@ namespace System.Formats.Nrbf; [DebuggerDisplay("{Value}")] public abstract class PrimitiveTypeRecord : PrimitiveTypeRecord { - private static TypeName? s_typeName; - private protected PrimitiveTypeRecord(T value) => Value = value; /// @@ -36,8 +34,7 @@ public abstract class PrimitiveTypeRecord : PrimitiveTypeRecord public new T Value { get; } /// - public override TypeName TypeName - => s_typeName ??= TypeName.Parse(typeof(T).FullName.AsSpan()).WithCoreLibAssemblyName(); + public override TypeName TypeName => TypeNameHelpers.GetPrimitiveTypeName(TypeNameHelpers.GetPrimitiveType()); internal override object? GetValue() => Value; } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index 9cabb1aa8bf2a5..c1d86fca8e3a56 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -12,8 +12,79 @@ namespace System.Formats.Nrbf.Utils; internal static class TypeNameHelpers { + // PrimitiveType does not define Object, we fake it as the last element. + internal const PrimitiveType ObjectPrimitiveType = (PrimitiveType)19; + private static readonly TypeName?[] s_PrimitiveTypeNames = new TypeName?[(int)ObjectPrimitiveType + 1]; + private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)ObjectPrimitiveType + 1]; private static AssemblyNameInfo? s_CoreLibAssemblyName; + internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) + { + Debug.Assert(primitiveType is not (PrimitiveType.None or PrimitiveType.Null)); + + TypeName? typeName = s_PrimitiveTypeNames[(int)primitiveType]; + if (typeName is null) + { + string fullName = primitiveType switch + { + PrimitiveType.Boolean => "System.Boolean", + PrimitiveType.Byte => "System.Byte", + PrimitiveType.SByte => "System.SByte", + PrimitiveType.Char => "System.Char", + PrimitiveType.Int16 => "System.Int16", + PrimitiveType.UInt16 => "System.UInt16", + PrimitiveType.Int32 => "System.Int32", + PrimitiveType.UInt32 => "System.UInt32", + PrimitiveType.Int64 => "System.Int64", + PrimitiveType.Single => "System.Single", + PrimitiveType.Double => "System.Double", + PrimitiveType.Decimal => "System.Decimal", + PrimitiveType.TimeSpan => "System.TimeSpan", + PrimitiveType.DateTime => "System.DateTime", + PrimitiveType.String => "System.String", + ObjectPrimitiveType => "System.Object", + _ => "System.UInt64", + }; + + s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); + } + return typeName; + } + + internal static TypeName GetPrimitiveSZArrayTypeName(PrimitiveType primitiveType) + { + TypeName? typeName = s_PrimitiveSZArrayTypeNames[(int)primitiveType]; + if (typeName is null) + { + s_PrimitiveSZArrayTypeNames[(int)primitiveType] = typeName = GetPrimitiveTypeName(primitiveType).MakeArrayTypeName(); + } + return typeName; + } + + internal static PrimitiveType GetPrimitiveType() + { + if (typeof(T) == typeof(bool)) return PrimitiveType.Boolean; + else if (typeof(T) == typeof(byte)) return PrimitiveType.Byte; + else if (typeof(T) == typeof(sbyte)) return PrimitiveType.SByte; + else if (typeof(T) == typeof(char)) return PrimitiveType.Char; + else if (typeof(T) == typeof(short)) return PrimitiveType.Int16; + else if (typeof(T) == typeof(ushort)) return PrimitiveType.UInt16; + else if (typeof(T) == typeof(int)) return PrimitiveType.Int32; + else if (typeof(T) == typeof(uint)) return PrimitiveType.UInt32; + else if (typeof(T) == typeof(long)) return PrimitiveType.Int64; + else if (typeof(T) == typeof(ulong)) return PrimitiveType.UInt64; + else if (typeof(T) == typeof(float)) return PrimitiveType.Single; + else if (typeof(T) == typeof(double)) return PrimitiveType.Double; + else if (typeof(T) == typeof(decimal)) return PrimitiveType.Decimal; + else if (typeof(T) == typeof(DateTime)) return PrimitiveType.DateTime; + else if (typeof(T) == typeof(TimeSpan)) return PrimitiveType.TimeSpan; + else + { + Debug.Assert(typeof(T) == typeof(string)); + return PrimitiveType.String; + } + } + internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, BinaryLibraryRecord libraryRecord, PayloadOptions payloadOptions) { if (libraryRecord.LibraryName is not null) @@ -53,19 +124,6 @@ internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadO internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) => systemType.WithAssemblyName(s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); - internal static TypeName BuildCoreLibArrayTypeName(this Type type, int arrayRank) - => WithCoreLibAssemblyName(BuildArrayTypeName(TypeName.Parse(type.FullName.AsSpan()), arrayRank)); - - internal static TypeName BuildArrayTypeName(this TypeName typeName, int arrayRank) - { - // In general, arrayRank == 1 may have two different meanings: - // - [] is a single dimension and zero-indexed array (SZArray) - // - [*] is single dimension, custom offset array. - // Custom offset arrays are not supported by design. - // That is why we don't call TypeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. - return arrayRank == 1 ? typeName.MakeArrayTypeName() : typeName.MakeArrayTypeName(arrayRank); - } - private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) { if (!TypeName.TryParse(rawName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions) From b37b8dadb2adf785f1c74f60993700ddf6a071fd Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 19 Jun 2024 18:26:12 +0200 Subject: [PATCH 04/18] add more tests and finish WithAssemblyName implementation --- .../Formats/Nrbf/BinaryLibraryRecord.cs | 1 - .../System/Reflection/Metadata/TypeName.cs | 32 +--- .../tests/Metadata/TypeNameTests.cs | 141 ++++++++++++++++-- 3 files changed, 136 insertions(+), 38 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs index 331d633c63263e..ccd39922e23fba 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/BinaryLibraryRecord.cs @@ -5,7 +5,6 @@ using System.Formats.Nrbf.Utils; using System.IO; using System.Reflection.Metadata; -using System.Runtime.Serialization; namespace System.Formats.Nrbf; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 27f460656ca697..3af9af9cb88784 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -466,34 +466,9 @@ public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) { return null; } - else if (oldName is not null && !ReferenceEquals(typeName.AssemblyName, oldName)) + else if (!ReferenceEquals(typeName.AssemblyName, oldName)) { - // There is nothing to update, do nothing. - return typeName; - } - - ImmutableArray genericArguments = typeName._genericArguments; - - if (typeName.IsConstructedGenericType) - { - bool needsNewArray = false; - for (int i = 0; i < typeName._genericArguments.Length; i++) - { - if (ReferenceEquals(typeName._genericArguments[i].AssemblyName, oldName)) - { - needsNewArray = true; - } - } - - if (needsNewArray) - { - ImmutableArray.Builder builder = ImmutableArray.CreateBuilder(typeName._genericArguments.Length); - for (int i = 0; i < typeName._genericArguments.Length; i++) - { - builder.Add(WithAssemblyName(typeName._genericArguments[i], oldName, newName)!); - } - genericArguments = builder.MoveToImmutable(); - } + return typeName; // There is nothing to update. } return new TypeName( @@ -501,7 +476,8 @@ public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) newName, elementOrGenericType: WithAssemblyName(typeName._elementOrGenericType, oldName, newName), declaringType: WithAssemblyName(typeName._declaringType, oldName, newName), - genericTypeArguments: genericArguments, + // We don't update the assembly names of generic arguments on purpose! + genericTypeArguments: typeName._genericArguments, rankOrModifier: typeName._rankOrModifier, nestedNameLength: typeName._nestedNameLength); } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index df21197ef065f5..cde7184f83b356 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -506,17 +506,140 @@ public void MakeGenericTypeName() } [Theory] - [InlineData("Namespace.Simple")] - [InlineData("Namespace.Simple, SomeAssembly")] - public void WithAssemblyName_SimpleNames(string name) + [InlineData("Simple")] + [InlineData("Pointer*")] + [InlineData("Pointer*******")] + [InlineData("ByRef&")] + [InlineData("SZArray[]")] + [InlineData("CustomOffsetArray[*]")] + [InlineData("MDArray[,]")] + [InlineData("Declaring+Nested")] + [InlineData("Declaring+Deep+Deeper+Nested")] + [InlineData("Generic[[GenericArg]]")] + public void WithAssemblyNameCanSetANewName(string fullName) { - TypeName simpleName = TypeName.Parse(name.AsSpan()); - Assert.Equal(name, simpleName.AssemblyQualifiedName); - AssemblyNameInfo assemblyName = new("Different"); + TypeName typeName = TypeName.Parse(fullName.AsSpan()); + Assert.Equal(fullName, typeName.FullName); + Assert.Equal(fullName, typeName.AssemblyQualifiedName); + Assert.Null(typeName.AssemblyName); - TypeName withDifferentAssemblyName = simpleName.WithAssemblyName(assemblyName); - Assert.Same(assemblyName, withDifferentAssemblyName.AssemblyName); - Assert.Equal("Namespace.Simple, Different", withDifferentAssemblyName.AssemblyQualifiedName); + VerifySimpleAssemblyNames(typeName, expectedAssemblyName: null); + + if (typeName.IsConstructedGenericType) + { + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + + AssemblyNameInfo newAssemblyName = new("NewName"); + TypeName withDifferentAssemblyName = typeName.WithAssemblyName(newAssemblyName); + + // ensure it does not mutate current instance, but returns a new one + Assert.NotSame(typeName, withDifferentAssemblyName); + // full name does not include assembly name and should remain unchanged + Assert.Equal(fullName, withDifferentAssemblyName.FullName); + // ensure it has not created a new copy of the immutable name + Assert.Same(newAssemblyName, withDifferentAssemblyName.AssemblyName); + // ensure the old AssemblyQualifiedName was not copied + Assert.Equal($"{fullName}, NewName", withDifferentAssemblyName.AssemblyQualifiedName); + + VerifySimpleAssemblyNames(withDifferentAssemblyName, expectedAssemblyName: newAssemblyName); + + if (typeName.IsConstructedGenericType) + { + // generic arguments remain unchanged on purpose + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + } + + [Theory] + [InlineData("Simple, AssemblyName")] + [InlineData("Pointer*, AssemblyName")] + [InlineData("Pointer*****, AssemblyName")] + [InlineData("ByRef&, AssemblyName")] + [InlineData("SZArray[], AssemblyName")] + [InlineData("CustomOffsetArray[*], AssemblyName")] + [InlineData("MDArray[,], AssemblyName")] + [InlineData("Declaring+Nested, AssemblyName")] + [InlineData("Declaring+Deep+Deeper+Nested, AssemblyName")] + [InlineData("Generic[[GenericArg]], AssemblyName")] + public void WithAssemblyNameCanReplaceExistingName(string assemblyQualifiedName) + { + TypeName typeName = TypeName.Parse(assemblyQualifiedName.AsSpan()); + Assert.NotEqual(assemblyQualifiedName, typeName.FullName); + Assert.Equal(assemblyQualifiedName, typeName.AssemblyQualifiedName); + + VerifySimpleAssemblyNames(typeName, typeName.AssemblyName); + + if (typeName.IsConstructedGenericType) + { + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + + AssemblyNameInfo differentAssemblyName = new("Different"); + TypeName withDifferentAssemblyName = typeName.WithAssemblyName(differentAssemblyName); + + VerifySimpleAssemblyNames(withDifferentAssemblyName, differentAssemblyName); + + if (typeName.IsConstructedGenericType) + { + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + } + + [Theory] + [InlineData("Simple, AssemblyName")] + [InlineData("Pointer*, AssemblyName")] + [InlineData("Pointer*****, AssemblyName")] + [InlineData("ByRef&, AssemblyName")] + [InlineData("SZArray[], AssemblyName")] + [InlineData("CustomOffsetArray[*], AssemblyName")] + [InlineData("MDArray[,], AssemblyName")] + [InlineData("Declaring+Nested, AssemblyName")] + [InlineData("Declaring+Deep+Deeper+Nested, AssemblyName")] + [InlineData("Generic[[GenericArg]], AssemblyName")] + public void WithAssemblyNameCanRemoveExistingName(string assemblyQualifiedName) + { + TypeName typeName = TypeName.Parse(assemblyQualifiedName.AsSpan()); + Assert.NotEqual(assemblyQualifiedName, typeName.FullName); + Assert.Equal(assemblyQualifiedName, typeName.AssemblyQualifiedName); + + VerifySimpleAssemblyNames(typeName, typeName.AssemblyName); + + if (typeName.IsConstructedGenericType) + { + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + + AssemblyNameInfo? noAssemblyName = null; + TypeName withDifferentAssemblyName = typeName.WithAssemblyName(noAssemblyName); + + VerifySimpleAssemblyNames(withDifferentAssemblyName, noAssemblyName); + + if (typeName.IsConstructedGenericType) + { + Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); + } + } + + private static void VerifySimpleAssemblyNames(TypeName typeName, AssemblyNameInfo? expectedAssemblyName) + { + Assert.Same(expectedAssemblyName, typeName.AssemblyName); + + TypeName elementTypeName = typeName; + while (elementTypeName.IsPointer || elementTypeName.IsByRef || elementTypeName.IsArray) + { + elementTypeName = elementTypeName.GetElementType(); + // the assembly name of the element type has to be updated + Assert.Same(expectedAssemblyName, elementTypeName.AssemblyName); + } + + TypeName declaringType = typeName; + while (declaringType.IsNested) + { + declaringType = declaringType.DeclaringType; + // the assembly name of the declaring type has to be updated + Assert.Same(expectedAssemblyName, declaringType.AssemblyName); + } } [Fact] From 6b933d9222d255512fe513bb339bdbb6a5d47d4c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 28 Jun 2024 13:17:21 +0200 Subject: [PATCH 05/18] finish the implementation --- .../Formats/Nrbf/SerializationRecord.cs | 2 +- .../src/Resources/Strings.resx | 3 +++ .../System/Reflection/Metadata/TypeName.cs | 21 +++++++++++++++++-- .../tests/Metadata/TypeNameTests.cs | 18 +++++++++++++--- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs index ccfce69124b706..751a932d8f8e08 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SerializationRecord.cs @@ -46,7 +46,7 @@ internal SerializationRecord() // others can't derive from this type /// /// /// This method ignores assembly names. - /// This method does NOT take into account member names or their genericTypes. + /// This method does NOT take into account member names or their types. /// /// The type to compare against. /// if the serialized type name match provided type; otherwise, . diff --git a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx index 963e4d0af9f8a3..bbc85441e3e008 100644 --- a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx @@ -432,4 +432,7 @@ The given assembly name was invalid. + + MakeGenericTypeName may only be called on a type name for which TypeName.IsSimple is true, for '{0}' it's not. + \ No newline at end of file diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 3af9af9cb88784..0017f19580b2ab 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -427,6 +427,7 @@ public int GetArrayRank() /// Returns a object representing an array of the current type, /// with the specified number of dimensions. /// + /// The number of dimensions for the array. This number must be more than zero and less than or equal to 32. /// /// A object representing an array of the current type, /// with the specified number of dimensions. @@ -453,10 +454,26 @@ public TypeName MakeArrayTypeName(int rank) /// public TypeName MakeByRefTypeName() => MakeElementTypeName(TypeNameParserHelpers.ByRef); + /// + /// Creates a new constructed generic type name. + /// + /// An array of type names to be used as generic arguments of the current simple type name. + /// + /// A representing the constructed type name formed by using the elements + /// of for the generic arguments of the current simple type name. + /// + /// The current type name is not simple. public TypeName MakeGenericTypeName(ImmutableArray typeArguments) - => new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, genericTypeArguments: typeArguments); + => IsSimple + ? new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, genericTypeArguments: typeArguments) + : throw new InvalidOperationException(SR.Format(SR.Arg_NotSimpleTypeName, FullName)); - // Nulls are allowed, as they allow for simply removing the name. + /// + /// Returns a object that represents the current type name with provided assembly name. + /// + /// + /// A object that represents the current type name with provided assembly name. + /// public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) => WithAssemblyName(this, AssemblyName, assemblyName)!; diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index cde7184f83b356..c2225ce634462d 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -500,9 +500,21 @@ public void MakeGenericTypeName() TypeName genericTypeNameDefinition = TypeName.Parse(genericTypeDefinition.AssemblyQualifiedName.AsSpan()); Type[] genericArgs = new Type[] { typeof(int), typeof(bool) }; - EnsureBasicMatch( - genericTypeNameDefinition.MakeGenericTypeName(genericArgs.Select(type => TypeName.Parse(type.AssemblyQualifiedName.AsSpan())).ToImmutableArray()), - genericTypeDefinition.MakeGenericType(genericArgs)); + ImmutableArray genericTypeNames = genericArgs.Select(type => TypeName.Parse(type.AssemblyQualifiedName.AsSpan())).ToImmutableArray(); + + TypeName genericTypeName = genericTypeNameDefinition.MakeGenericTypeName(genericTypeNames); + Type genericType = genericTypeDefinition.MakeGenericType(genericArgs); + + EnsureBasicMatch(genericTypeName, genericType); + + TypeName szArrayTypeName = genericTypeNameDefinition.MakeArrayTypeName(); + Assert.Throws(() => szArrayTypeName.MakeGenericTypeName(genericTypeNames)); + TypeName mdArrayTypeName = genericTypeNameDefinition.MakeArrayTypeName(3); + Assert.Throws(() => mdArrayTypeName.MakeGenericTypeName(genericTypeNames)); + TypeName pointerTypeName = genericTypeNameDefinition.MakePointerTypeName(); + Assert.Throws(() => pointerTypeName.MakeGenericTypeName(genericTypeNames)); + TypeName byRefTypeName = genericTypeNameDefinition.MakeByRefTypeName(); + Assert.Throws(() => byRefTypeName.MakeGenericTypeName(genericTypeNames)); } [Theory] From 1736b24c0e254ba02ee86fab4acdf891262123d6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 28 Jun 2024 13:46:48 +0200 Subject: [PATCH 06/18] add missing IntPtr and UIntPtr support (discovered once I've re-enabled BF tests in main and synced the branch) --- .../src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index c1d86fca8e3a56..c981c2971ddce4 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -12,10 +12,12 @@ namespace System.Formats.Nrbf.Utils; internal static class TypeNameHelpers { - // PrimitiveType does not define Object, we fake it as the last element. + // PrimitiveType does not define Object, IntPtr or UIntPtr internal const PrimitiveType ObjectPrimitiveType = (PrimitiveType)19; - private static readonly TypeName?[] s_PrimitiveTypeNames = new TypeName?[(int)ObjectPrimitiveType + 1]; - private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)ObjectPrimitiveType + 1]; + internal const PrimitiveType IntPtrPrimitiveType = (PrimitiveType)20; + internal const PrimitiveType UIntPtrPrimitiveType = (PrimitiveType)21; + private static readonly TypeName?[] s_PrimitiveTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; + private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; private static AssemblyNameInfo? s_CoreLibAssemblyName; internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) @@ -43,6 +45,8 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) PrimitiveType.DateTime => "System.DateTime", PrimitiveType.String => "System.String", ObjectPrimitiveType => "System.Object", + IntPtrPrimitiveType => "System.IntPtr", + UIntPtrPrimitiveType => "System.UIntPtr", _ => "System.UInt64", }; @@ -78,6 +82,8 @@ internal static PrimitiveType GetPrimitiveType() else if (typeof(T) == typeof(decimal)) return PrimitiveType.Decimal; else if (typeof(T) == typeof(DateTime)) return PrimitiveType.DateTime; else if (typeof(T) == typeof(TimeSpan)) return PrimitiveType.TimeSpan; + else if (typeof(T) == typeof(IntPtr)) return IntPtrPrimitiveType; + else if (typeof(T) == typeof(UIntPtr)) return UIntPtrPrimitiveType; else { Debug.Assert(typeof(T) == typeof(string)); From a54e55fceabdcd0b603e3a471f8741a4b0c4296d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 28 Jun 2024 13:50:44 +0200 Subject: [PATCH 07/18] use TypeNameMatches in more places --- .../SystemClassWithMembersAndTypesRecord.cs | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SystemClassWithMembersAndTypesRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SystemClassWithMembersAndTypesRecord.cs index 93095bc32b0007..05d38ec736f10c 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SystemClassWithMembersAndTypesRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/SystemClassWithMembersAndTypesRecord.cs @@ -38,6 +38,11 @@ internal override (AllowedRecordTypes allowed, PrimitiveType primitiveType) GetN // to get a single primitive value! internal SerializationRecord TryToMapToUserFriendly() { + if (!TypeName.IsSimple) + { + return this; + } + if (MemberValues.Count == 1) { if (HasMember("m_value")) @@ -45,18 +50,18 @@ internal SerializationRecord TryToMapToUserFriendly() return MemberValues[0] switch { // there can be a value match, but no TypeName match - bool value when TypeName.FullName == typeof(bool).FullName => Create(value), - byte value when TypeName.FullName == typeof(byte).FullName => Create(value), - sbyte value when TypeName.FullName == typeof(sbyte).FullName => Create(value), - char value when TypeName.FullName == typeof(char).FullName => Create(value), - short value when TypeName.FullName == typeof(short).FullName => Create(value), - ushort value when TypeName.FullName == typeof(ushort).FullName => Create(value), - int value when TypeName.FullName == typeof(int).FullName => Create(value), - uint value when TypeName.FullName == typeof(uint).FullName => Create(value), - long value when TypeName.FullName == typeof(long).FullName => Create(value), - ulong value when TypeName.FullName == typeof(ulong).FullName => Create(value), - float value when TypeName.FullName == typeof(float).FullName => Create(value), - double value when TypeName.FullName == typeof(double).FullName => Create(value), + bool value when TypeNameMatches(typeof(bool)) => Create(value), + byte value when TypeNameMatches(typeof(byte)) => Create(value), + sbyte value when TypeNameMatches(typeof(sbyte)) => Create(value), + char value when TypeNameMatches(typeof(char)) => Create(value), + short value when TypeNameMatches(typeof(short)) => Create(value), + ushort value when TypeNameMatches(typeof(ushort)) => Create(value), + int value when TypeNameMatches(typeof(int)) => Create(value), + uint value when TypeNameMatches(typeof(uint)) => Create(value), + long value when TypeNameMatches(typeof(long)) => Create(value), + ulong value when TypeNameMatches(typeof(ulong)) => Create(value), + float value when TypeNameMatches(typeof(float)) => Create(value), + double value when TypeNameMatches(typeof(double)) => Create(value), _ => this }; } @@ -65,12 +70,12 @@ internal SerializationRecord TryToMapToUserFriendly() return MemberValues[0] switch { // there can be a value match, but no TypeName match - long value when TypeName.FullName == typeof(IntPtr).FullName => Create(new IntPtr(value)), - ulong value when TypeName.FullName == typeof(UIntPtr).FullName => Create(new UIntPtr(value)), + long value when TypeNameMatches(typeof(IntPtr)) => Create(new IntPtr(value)), + ulong value when TypeNameMatches(typeof(UIntPtr)) => Create(new UIntPtr(value)), _ => this }; } - else if (HasMember("_ticks") && MemberValues[0] is long ticks && TypeName.FullName == typeof(TimeSpan).FullName) + else if (HasMember("_ticks") && MemberValues[0] is long ticks && TypeNameMatches(typeof(TimeSpan))) { return Create(new TimeSpan(ticks)); } From 9a0cd5819f8d5367e94aeb0da9d98e6fb5ea51f3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 2 Aug 2024 12:33:11 +0200 Subject: [PATCH 08/18] address API review feedback --- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 2 +- .../System/Formats/Nrbf/Utils/ThrowHelper.cs | 3 + .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 40 ++++- .../tests/TypeMatchTests.cs | 6 + .../ref/System.Reflection.Metadata.cs | 6 +- .../src/Resources/Strings.resx | 3 + .../System/Reflection/Metadata/TypeName.cs | 70 ++++----- .../Metadata/TypeNameParserHelpers.cs | 2 +- .../tests/Metadata/TypeNameTests.cs | 137 ++---------------- 9 files changed, 105 insertions(+), 164 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 16d35b0fc9b5e6..220ab322d7a311 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -160,7 +160,7 @@ internal TypeName GetArrayTypeName(ArrayInfo arrayInfo) // Custom offset arrays are not supported by design, so in our case it's always SZArray. // That is why we don't call TypeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. return arrayInfo.Rank == 1 - ? elementTypeName.MakeArrayTypeName() + ? elementTypeName.MakeSZArrayTypeName() : elementTypeName.MakeArrayTypeName(arrayInfo.Rank); } } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs index 3689138fa14b13..f096bfc736098e 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs @@ -14,6 +14,9 @@ internal static void ThrowInvalidValue(object value) internal static void ThrowInvalidReference() => throw new SerializationException(SR.Serialization_InvalidReference); + internal static void ThrowInvalidTypeName(string name) + => throw new SerializationException(SR.Format(SR.Serialization_InvalidTypeName, name)); + internal static void ThrowUnexpectedNullRecordCount() => throw new SerializationException(SR.Serialization_UnexpectedNullRecordCount); diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index c981c2971ddce4..b0cce318af3564 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -20,6 +20,9 @@ internal static class TypeNameHelpers private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; private static AssemblyNameInfo? s_CoreLibAssemblyName; + internal static AssemblyNameInfo CoreLibAssemblyName + => s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan()); + internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) { Debug.Assert(primitiveType is not (PrimitiveType.None or PrimitiveType.Null)); @@ -50,7 +53,7 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) _ => "System.UInt64", }; - s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); + s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.CreateSimpleTypeName(fullName, assemblyName: CoreLibAssemblyName); } return typeName; } @@ -60,7 +63,7 @@ internal static TypeName GetPrimitiveSZArrayTypeName(PrimitiveType primitiveType TypeName? typeName = s_PrimitiveSZArrayTypeNames[(int)primitiveType]; if (typeName is null) { - s_PrimitiveSZArrayTypeNames[(int)primitiveType] = typeName = GetPrimitiveTypeName(primitiveType).MakeArrayTypeName(); + s_PrimitiveSZArrayTypeNames[(int)primitiveType] = typeName = GetPrimitiveTypeName(primitiveType).MakeSZArrayTypeName(); } return typeName; } @@ -128,7 +131,38 @@ internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadO .WithCoreLibAssemblyName(); // We know it's a System Record, so we set the LibraryName to CoreLib internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); + => systemType.WithAssemblyName(CoreLibAssemblyName); + + private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInfo assemblyName) + { + if (!typeName.IsSimple) + { + if (typeName.IsArray) + { + TypeName newElementType = typeName.GetElementType().WithAssemblyName(assemblyName); + + return typeName.IsSZArray + ? newElementType.MakeSZArrayTypeName() + : newElementType.MakeArrayTypeName(typeName.GetArrayRank()); + } + else if (typeName.IsConstructedGenericType) + { + TypeName newGenericTypeDefinition = typeName.GetGenericTypeDefinition().WithAssemblyName(assemblyName); + + // We don't change the assembly name of generic arguments on purpose. + return newGenericTypeDefinition.MakeGenericTypeName(typeName.GetGenericArguments()); + } + else + { + // BinaryFormatter can not serialize pointers or references. + ThrowHelper.ThrowInvalidTypeName(typeName.FullName); + } + } + + TypeName? newDeclaringType = typeName.IsNested ? typeName.DeclaringType.WithAssemblyName(assemblyName) : null; + + return TypeName.CreateSimpleTypeName(typeName.FullName, newDeclaringType, assemblyName); + } private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) { diff --git a/src/libraries/System.Formats.Nrbf/tests/TypeMatchTests.cs b/src/libraries/System.Formats.Nrbf/tests/TypeMatchTests.cs index 7bf48eab616899..6a981197f82f1c 100644 --- a/src/libraries/System.Formats.Nrbf/tests/TypeMatchTests.cs +++ b/src/libraries/System.Formats.Nrbf/tests/TypeMatchTests.cs @@ -302,6 +302,12 @@ private static void Verify(T input) where T : notnull Assert.True(one.TypeNameMatches(typeof(T))); + Assert.Equal(typeof(T).GetTypeFullNameIncludingTypeForwards(), one.TypeName.FullName); + if (typeof(T) != typeof(TimeSpan)) // TimeSpan is missing type forwards + { + Assert.Equal(typeof(T).GetAssemblyNameIncludingTypeForwards(), one.TypeName.AssemblyName!.FullName); + } + foreach (Type type in PrimitiveTypes) { Assert.Equal(typeof(T) == type, one.TypeNameMatches(type)); diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index b01b1e964591a4..7e23134830769b 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2427,7 +2427,7 @@ public sealed partial class TypeName internal TypeName() { } public string AssemblyQualifiedName { get { throw null; } } public AssemblyNameInfo? AssemblyName { get { throw null; } } - public System.Reflection.Metadata.TypeName? DeclaringType { get { throw null; } } + public System.Reflection.Metadata.TypeName DeclaringType { get { throw null; } } public string FullName { get { throw null; } } public bool IsArray { get { throw null; } } public bool IsByRef { get { throw null; } } @@ -2438,6 +2438,7 @@ internal TypeName() { } public bool IsSZArray { get { throw null; } } public bool IsVariableBoundArrayType { get { throw null; } } public string Name { get { throw null; } } + public static System.Reflection.Metadata.TypeName CreateSimpleTypeName(string metadataName, System.Reflection.Metadata.TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) { throw null; } public static System.Reflection.Metadata.TypeName Parse(System.ReadOnlySpan typeName, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } public static bool TryParse(System.ReadOnlySpan typeName, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Reflection.Metadata.TypeName? result, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } public int GetArrayRank() { throw null; } @@ -2445,12 +2446,11 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName GetGenericTypeDefinition() { throw null; } public System.Reflection.Metadata.TypeName GetElementType() { throw null; } public int GetNodeCount() { throw null; } - public System.Reflection.Metadata.TypeName MakeArrayTypeName() { throw null; } + public System.Reflection.Metadata.TypeName MakeSZArrayTypeName() { throw null; } public System.Reflection.Metadata.TypeName MakeArrayTypeName(int rank) { throw null; } public System.Reflection.Metadata.TypeName MakeByRefTypeName() { throw null; } public System.Reflection.Metadata.TypeName MakeGenericTypeName(System.Collections.Immutable.ImmutableArray typeArguments) { throw null; } public System.Reflection.Metadata.TypeName MakePointerTypeName() { throw null; } - public System.Reflection.Metadata.TypeName WithAssemblyName(AssemblyNameInfo assemblyName) { throw null; } } public sealed partial class TypeNameParseOptions { diff --git a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx index bbc85441e3e008..64264b093c0135 100644 --- a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx @@ -435,4 +435,7 @@ MakeGenericTypeName may only be called on a type name for which TypeName.IsSimple is true, for '{0}' it's not. + + The value cannot be an empty string. + \ No newline at end of file diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 0017f19580b2ab..a86a04f93dc4e6 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -413,6 +413,41 @@ public int GetArrayRank() #endif #if SYSTEM_REFLECTION_METADATA + /// + /// Creates a new object that represents given name with provided assembly name. + /// + /// Unescaped full name. + /// Declaring type name. + /// Assembly name. + /// Created simple name. + /// is . + /// Provided type name was invalid. For example, it was empty or escaped. + public static TypeName CreateSimpleTypeName(string metadataName, TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) + { +#if NET + ArgumentNullException.ThrowIfNullOrEmpty(nameof(metadataName)); + bool containsEscapeCharacter = metadataName.Contains(TypeNameParserHelpers.EscapeCharacter); +#else + if (string.IsNullOrEmpty(metadataName)) + { + throw metadataName is null + ? throw new ArgumentNullException(nameof(metadataName)) + : throw new ArgumentException(SR.Argument_EmptyString, nameof(metadataName)); + } + bool containsEscapeCharacter = metadataName.IndexOf(TypeNameParserHelpers.EscapeCharacter) >= 0; +#endif + if (containsEscapeCharacter) + { + throw new ArgumentException(SR.Argument_InvalidTypeName); + } + + return new TypeName(fullName: metadataName, + assemblyName: assemblyName, + elementOrGenericType: null, + declaringType: declaringType, + genericTypeArguments: ImmutableArray.Empty); + } + /// /// Returns a object representing a one-dimensional array /// of the current type, with a lower bound of zero. @@ -421,7 +456,7 @@ public int GetArrayRank() /// A object representing a one-dimensional array /// of the current type, with a lower bound of zero. /// - public TypeName MakeArrayTypeName() => MakeElementTypeName(TypeNameParserHelpers.SZArray); + public TypeName MakeSZArrayTypeName() => MakeElementTypeName(TypeNameParserHelpers.SZArray); /// /// Returns a object representing an array of the current type, @@ -465,40 +500,9 @@ public TypeName MakeArrayTypeName(int rank) /// The current type name is not simple. public TypeName MakeGenericTypeName(ImmutableArray typeArguments) => IsSimple - ? new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, genericTypeArguments: typeArguments) + ? new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, declaringType: _declaringType, genericTypeArguments: typeArguments) : throw new InvalidOperationException(SR.Format(SR.Arg_NotSimpleTypeName, FullName)); - /// - /// Returns a object that represents the current type name with provided assembly name. - /// - /// - /// A object that represents the current type name with provided assembly name. - /// - public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) - => WithAssemblyName(this, AssemblyName, assemblyName)!; - - private static TypeName? WithAssemblyName(TypeName? typeName, AssemblyNameInfo? oldName, AssemblyNameInfo? newName) - { - if (typeName is null) - { - return null; - } - else if (!ReferenceEquals(typeName.AssemblyName, oldName)) - { - return typeName; // There is nothing to update. - } - - return new TypeName( - fullName: typeName._fullName, - newName, - elementOrGenericType: WithAssemblyName(typeName._elementOrGenericType, oldName, newName), - declaringType: WithAssemblyName(typeName._declaringType, oldName, newName), - // We don't update the assembly names of generic arguments on purpose! - genericTypeArguments: typeName._genericArguments, - rankOrModifier: typeName._rankOrModifier, - nestedNameLength: typeName._nestedNameLength); - } - private TypeName MakeElementTypeName(sbyte rankOrModifier) => new TypeName( fullName: null, diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 2fa33ea1dfe3bf..b268ba4a820996 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -14,7 +14,7 @@ internal static class TypeNameParserHelpers internal const sbyte SZArray = -1; internal const sbyte Pointer = -2; internal const sbyte ByRef = -3; - private const char EscapeCharacter = '\\'; + internal const char EscapeCharacter = '\\'; #if NET8_0_OR_GREATER private static readonly SearchValues s_endOfFullTypeNameDelimitersSearchValues = SearchValues.Create("[]&*,+\\"); #endif diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index c2225ce634462d..68c266da4b9bfe 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -486,7 +486,7 @@ public void GetNodeCountReturnsExpectedValue(Type type, int expected) if (!type.IsByRef) { - EnsureBasicMatch(parsed.MakeArrayTypeName(), type.MakeArrayType()); + EnsureBasicMatch(parsed.MakeSZArrayTypeName(), type.MakeArrayType()); EnsureBasicMatch(parsed.MakeArrayTypeName(3), type.MakeArrayType(3)); EnsureBasicMatch(parsed.MakeByRefTypeName(), type.MakeByRefType()); EnsureBasicMatch(parsed.MakePointerTypeName(), type.MakePointerType()); @@ -507,7 +507,7 @@ public void MakeGenericTypeName() EnsureBasicMatch(genericTypeName, genericType); - TypeName szArrayTypeName = genericTypeNameDefinition.MakeArrayTypeName(); + TypeName szArrayTypeName = genericTypeNameDefinition.MakeSZArrayTypeName(); Assert.Throws(() => szArrayTypeName.MakeGenericTypeName(genericTypeNames)); TypeName mdArrayTypeName = genericTypeNameDefinition.MakeArrayTypeName(3); Assert.Throws(() => mdArrayTypeName.MakeGenericTypeName(genericTypeNames)); @@ -528,131 +528,22 @@ public void MakeGenericTypeName() [InlineData("Declaring+Nested")] [InlineData("Declaring+Deep+Deeper+Nested")] [InlineData("Generic[[GenericArg]]")] - public void WithAssemblyNameCanSetANewName(string fullName) + public void CreateSimpleTypeNameTreatsEveryInputAsUnescapedSimpleFullName(string input) { - TypeName typeName = TypeName.Parse(fullName.AsSpan()); - Assert.Equal(fullName, typeName.FullName); - Assert.Equal(fullName, typeName.AssemblyQualifiedName); - Assert.Null(typeName.AssemblyName); - - VerifySimpleAssemblyNames(typeName, expectedAssemblyName: null); - - if (typeName.IsConstructedGenericType) - { - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } - - AssemblyNameInfo newAssemblyName = new("NewName"); - TypeName withDifferentAssemblyName = typeName.WithAssemblyName(newAssemblyName); - - // ensure it does not mutate current instance, but returns a new one - Assert.NotSame(typeName, withDifferentAssemblyName); - // full name does not include assembly name and should remain unchanged - Assert.Equal(fullName, withDifferentAssemblyName.FullName); - // ensure it has not created a new copy of the immutable name - Assert.Same(newAssemblyName, withDifferentAssemblyName.AssemblyName); - // ensure the old AssemblyQualifiedName was not copied - Assert.Equal($"{fullName}, NewName", withDifferentAssemblyName.AssemblyQualifiedName); - - VerifySimpleAssemblyNames(withDifferentAssemblyName, expectedAssemblyName: newAssemblyName); - - if (typeName.IsConstructedGenericType) - { - // generic arguments remain unchanged on purpose - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } - } - - [Theory] - [InlineData("Simple, AssemblyName")] - [InlineData("Pointer*, AssemblyName")] - [InlineData("Pointer*****, AssemblyName")] - [InlineData("ByRef&, AssemblyName")] - [InlineData("SZArray[], AssemblyName")] - [InlineData("CustomOffsetArray[*], AssemblyName")] - [InlineData("MDArray[,], AssemblyName")] - [InlineData("Declaring+Nested, AssemblyName")] - [InlineData("Declaring+Deep+Deeper+Nested, AssemblyName")] - [InlineData("Generic[[GenericArg]], AssemblyName")] - public void WithAssemblyNameCanReplaceExistingName(string assemblyQualifiedName) - { - TypeName typeName = TypeName.Parse(assemblyQualifiedName.AsSpan()); - Assert.NotEqual(assemblyQualifiedName, typeName.FullName); - Assert.Equal(assemblyQualifiedName, typeName.AssemblyQualifiedName); - - VerifySimpleAssemblyNames(typeName, typeName.AssemblyName); - - if (typeName.IsConstructedGenericType) - { - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } - - AssemblyNameInfo differentAssemblyName = new("Different"); - TypeName withDifferentAssemblyName = typeName.WithAssemblyName(differentAssemblyName); - - VerifySimpleAssemblyNames(withDifferentAssemblyName, differentAssemblyName); - - if (typeName.IsConstructedGenericType) - { - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } + TypeName typeName = TypeName.CreateSimpleTypeName(input); + + Assert.True(typeName.IsSimple); + Assert.False(typeName.IsArray); + Assert.False(typeName.IsConstructedGenericType); + Assert.False(typeName.IsPointer); + Assert.False(typeName.IsByRef); + Assert.False(typeName.IsNested); } [Theory] - [InlineData("Simple, AssemblyName")] - [InlineData("Pointer*, AssemblyName")] - [InlineData("Pointer*****, AssemblyName")] - [InlineData("ByRef&, AssemblyName")] - [InlineData("SZArray[], AssemblyName")] - [InlineData("CustomOffsetArray[*], AssemblyName")] - [InlineData("MDArray[,], AssemblyName")] - [InlineData("Declaring+Nested, AssemblyName")] - [InlineData("Declaring+Deep+Deeper+Nested, AssemblyName")] - [InlineData("Generic[[GenericArg]], AssemblyName")] - public void WithAssemblyNameCanRemoveExistingName(string assemblyQualifiedName) - { - TypeName typeName = TypeName.Parse(assemblyQualifiedName.AsSpan()); - Assert.NotEqual(assemblyQualifiedName, typeName.FullName); - Assert.Equal(assemblyQualifiedName, typeName.AssemblyQualifiedName); - - VerifySimpleAssemblyNames(typeName, typeName.AssemblyName); - - if (typeName.IsConstructedGenericType) - { - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } - - AssemblyNameInfo? noAssemblyName = null; - TypeName withDifferentAssemblyName = typeName.WithAssemblyName(noAssemblyName); - - VerifySimpleAssemblyNames(withDifferentAssemblyName, noAssemblyName); - - if (typeName.IsConstructedGenericType) - { - Assert.All(typeName.GetGenericArguments(), genericArg => Assert.Null(genericArg.AssemblyName)); - } - } - - private static void VerifySimpleAssemblyNames(TypeName typeName, AssemblyNameInfo? expectedAssemblyName) - { - Assert.Same(expectedAssemblyName, typeName.AssemblyName); - - TypeName elementTypeName = typeName; - while (elementTypeName.IsPointer || elementTypeName.IsByRef || elementTypeName.IsArray) - { - elementTypeName = elementTypeName.GetElementType(); - // the assembly name of the element type has to be updated - Assert.Same(expectedAssemblyName, elementTypeName.AssemblyName); - } - - TypeName declaringType = typeName; - while (declaringType.IsNested) - { - declaringType = declaringType.DeclaringType; - // the assembly name of the declaring type has to be updated - Assert.Same(expectedAssemblyName, declaringType.AssemblyName); - } - } + [MemberData(nameof(GetTypesThatRequireEscaping))] + public void CreateSimpleTypeNameThrowsForEscapedInput(Type type) + => Assert.Throws(() => TypeName.CreateSimpleTypeName(type.FullName)); [Fact] public void IsSimpleReturnsTrueForNestedNonGenericTypes() From aabe51b8bfb7e2ab7d90d1672e4feb56f91315af Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 2 Aug 2024 15:20:48 +0200 Subject: [PATCH 09/18] remove CreateSimpleTypeName, introduce MakeSimpleTypeNameThrowsForNonSimpleNames: - all TypeName instances remain always valid and escaped - it's impossible to detect all sequences that are escaped --- .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 10 ++--- .../ref/System.Reflection.Metadata.cs | 2 +- .../System/Reflection/Metadata/TypeName.cs | 39 +++++++------------ .../Metadata/TypeNameParserHelpers.cs | 10 ++++- .../tests/Metadata/TypeNameTests.cs | 37 +++++++++++------- 5 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index b0cce318af3564..b2e68a88a7478c 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -20,9 +20,6 @@ internal static class TypeNameHelpers private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; private static AssemblyNameInfo? s_CoreLibAssemblyName; - internal static AssemblyNameInfo CoreLibAssemblyName - => s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan()); - internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) { Debug.Assert(primitiveType is not (PrimitiveType.None or PrimitiveType.Null)); @@ -53,7 +50,7 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) _ => "System.UInt64", }; - s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.CreateSimpleTypeName(fullName, assemblyName: CoreLibAssemblyName); + s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); } return typeName; } @@ -131,7 +128,7 @@ internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadO .WithCoreLibAssemblyName(); // We know it's a System Record, so we set the LibraryName to CoreLib internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(CoreLibAssemblyName); + => systemType.WithAssemblyName(s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInfo assemblyName) { @@ -160,8 +157,7 @@ private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInf } TypeName? newDeclaringType = typeName.IsNested ? typeName.DeclaringType.WithAssemblyName(assemblyName) : null; - - return TypeName.CreateSimpleTypeName(typeName.FullName, newDeclaringType, assemblyName); + return typeName.MakeSimpleTypeName(newDeclaringType, assemblyName); } private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index 7e23134830769b..f9b35a68bbb4a0 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2438,7 +2438,6 @@ internal TypeName() { } public bool IsSZArray { get { throw null; } } public bool IsVariableBoundArrayType { get { throw null; } } public string Name { get { throw null; } } - public static System.Reflection.Metadata.TypeName CreateSimpleTypeName(string metadataName, System.Reflection.Metadata.TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) { throw null; } public static System.Reflection.Metadata.TypeName Parse(System.ReadOnlySpan typeName, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } public static bool TryParse(System.ReadOnlySpan typeName, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Reflection.Metadata.TypeName? result, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } public int GetArrayRank() { throw null; } @@ -2451,6 +2450,7 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName MakeByRefTypeName() { throw null; } public System.Reflection.Metadata.TypeName MakeGenericTypeName(System.Collections.Immutable.ImmutableArray typeArguments) { throw null; } public System.Reflection.Metadata.TypeName MakePointerTypeName() { throw null; } + public System.Reflection.Metadata.TypeName MakeSimpleTypeName(System.Reflection.Metadata.TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) { throw null; } } public sealed partial class TypeNameParseOptions { diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index a86a04f93dc4e6..e5e1295239f476 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -414,37 +414,23 @@ public int GetArrayRank() #if SYSTEM_REFLECTION_METADATA /// - /// Creates a new object that represents given name with provided assembly name. + /// Creates a new object that represents current simple name with provided assembly name and declaring type. /// - /// Unescaped full name. /// Declaring type name. /// Assembly name. /// Created simple name. - /// is . - /// Provided type name was invalid. For example, it was empty or escaped. - public static TypeName CreateSimpleTypeName(string metadataName, TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) + /// The current type name is not simple. + public TypeName MakeSimpleTypeName(TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) { -#if NET - ArgumentNullException.ThrowIfNullOrEmpty(nameof(metadataName)); - bool containsEscapeCharacter = metadataName.Contains(TypeNameParserHelpers.EscapeCharacter); -#else - if (string.IsNullOrEmpty(metadataName)) - { - throw metadataName is null - ? throw new ArgumentNullException(nameof(metadataName)) - : throw new ArgumentException(SR.Argument_EmptyString, nameof(metadataName)); - } - bool containsEscapeCharacter = metadataName.IndexOf(TypeNameParserHelpers.EscapeCharacter) >= 0; -#endif - if (containsEscapeCharacter) + if (!IsSimple) { - throw new ArgumentException(SR.Argument_InvalidTypeName); + TypeNameParserHelpers.ThrowInvalidOperation_NotSimpleName(FullName); } - return new TypeName(fullName: metadataName, + return new TypeName(fullName: FullName, assemblyName: assemblyName, elementOrGenericType: null, - declaringType: declaringType, + declaringType: IsNested ? declaringType ?? DeclaringType : null, genericTypeArguments: ImmutableArray.Empty); } @@ -499,9 +485,14 @@ public TypeName MakeArrayTypeName(int rank) /// /// The current type name is not simple. public TypeName MakeGenericTypeName(ImmutableArray typeArguments) - => IsSimple - ? new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, declaringType: _declaringType, genericTypeArguments: typeArguments) - : throw new InvalidOperationException(SR.Format(SR.Arg_NotSimpleTypeName, FullName)); + { + if (!IsSimple) + { + TypeNameParserHelpers.ThrowInvalidOperation_NotSimpleName(FullName); + } + + return new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, declaringType: _declaringType, genericTypeArguments: typeArguments); + } private TypeName MakeElementTypeName(sbyte rankOrModifier) => new TypeName( diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index b268ba4a820996..ef0a9e702a1a22 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -14,7 +14,7 @@ internal static class TypeNameParserHelpers internal const sbyte SZArray = -1; internal const sbyte Pointer = -2; internal const sbyte ByRef = -3; - internal const char EscapeCharacter = '\\'; + private const char EscapeCharacter = '\\'; #if NET8_0_OR_GREATER private static readonly SearchValues s_endOfFullTypeNameDelimitersSearchValues = SearchValues.Create("[]&*,+\\"); #endif @@ -388,5 +388,13 @@ internal static void ThrowInvalidOperation_HasToBeArrayClass() throw new InvalidOperationException(); #endif } + +#if SYSTEM_REFLECTION_METADATA + [DoesNotReturn] + internal static void ThrowInvalidOperation_NotSimpleName(string fullName) + { + throw new InvalidOperationException(SR.Format(SR.Arg_NotSimpleTypeName, fullName)); + } +#endif } } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 68c266da4b9bfe..dc87255bb376b1 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -528,22 +528,31 @@ public void MakeGenericTypeName() [InlineData("Declaring+Nested")] [InlineData("Declaring+Deep+Deeper+Nested")] [InlineData("Generic[[GenericArg]]")] - public void CreateSimpleTypeNameTreatsEveryInputAsUnescapedSimpleFullName(string input) + public void MakeSimpleTypeNameThrowsForNonSimpleNames(string input) { - TypeName typeName = TypeName.CreateSimpleTypeName(input); - - Assert.True(typeName.IsSimple); - Assert.False(typeName.IsArray); - Assert.False(typeName.IsConstructedGenericType); - Assert.False(typeName.IsPointer); - Assert.False(typeName.IsByRef); - Assert.False(typeName.IsNested); - } + TypeName typeName = TypeName.Parse(input.AsSpan()); - [Theory] - [MemberData(nameof(GetTypesThatRequireEscaping))] - public void CreateSimpleTypeNameThrowsForEscapedInput(Type type) - => Assert.Throws(() => TypeName.CreateSimpleTypeName(type.FullName)); + if (typeName.IsSimple) + { + AssemblyNameInfo assemblyName = new("MyName"); + TypeName simple = typeName.MakeSimpleTypeName(assemblyName: assemblyName); + + Assert.Equal(typeName.FullName, simple.FullName); // full name has not changed + Assert.NotEqual(typeName.AssemblyQualifiedName, simple.AssemblyQualifiedName); + Assert.Equal($"{typeName.FullName}, {assemblyName.FullName}", simple.AssemblyQualifiedName); + + Assert.True(simple.IsSimple); + Assert.False(simple.IsArray); + Assert.False(simple.IsConstructedGenericType); + Assert.False(simple.IsPointer); + Assert.False(simple.IsByRef); + Assert.Equal(typeName.IsNested, simple.IsNested); + } + else + { + Assert.Throws(() => typeName.MakeSimpleTypeName()); + } + } [Fact] public void IsSimpleReturnsTrueForNestedNonGenericTypes() From e207e0f4e76e5b6e6ebfd8e29ae4aa8e0bf52181 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 6 Aug 2024 16:14:16 +0200 Subject: [PATCH 10/18] handle nested names properly --- .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 3 +- .../ref/System.Reflection.Metadata.cs | 2 +- .../System/Reflection/Metadata/TypeName.cs | 11 ++-- .../tests/Metadata/TypeNameTests.cs | 58 ++++++++++++++++++- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index b2e68a88a7478c..a2bf3774d360f5 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -156,8 +156,7 @@ private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInf } } - TypeName? newDeclaringType = typeName.IsNested ? typeName.DeclaringType.WithAssemblyName(assemblyName) : null; - return typeName.MakeSimpleTypeName(newDeclaringType, assemblyName); + return typeName.MakeSimpleTypeName(assemblyName); } private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index f9b35a68bbb4a0..3b2994c1aa6401 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2450,7 +2450,7 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName MakeByRefTypeName() { throw null; } public System.Reflection.Metadata.TypeName MakeGenericTypeName(System.Collections.Immutable.ImmutableArray typeArguments) { throw null; } public System.Reflection.Metadata.TypeName MakePointerTypeName() { throw null; } - public System.Reflection.Metadata.TypeName MakeSimpleTypeName(System.Reflection.Metadata.TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) { throw null; } + public System.Reflection.Metadata.TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) { throw null; } } public sealed partial class TypeNameParseOptions { diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index e5e1295239f476..99dcc953d000a1 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -414,23 +414,26 @@ public int GetArrayRank() #if SYSTEM_REFLECTION_METADATA /// - /// Creates a new object that represents current simple name with provided assembly name and declaring type. + /// Creates a new object that represents current simple name with provided assembly name. /// - /// Declaring type name. /// Assembly name. /// Created simple name. /// The current type name is not simple. - public TypeName MakeSimpleTypeName(TypeName? declaringType = null, AssemblyNameInfo? assemblyName = null) + public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) { if (!IsSimple) { TypeNameParserHelpers.ThrowInvalidOperation_NotSimpleName(FullName); } + TypeName? declaringType = IsNested + ? DeclaringType.MakeSimpleTypeName(assemblyName) + : null; + return new TypeName(fullName: FullName, assemblyName: assemblyName, elementOrGenericType: null, - declaringType: IsNested ? declaringType ?? DeclaringType : null, + declaringType: declaringType, genericTypeArguments: ImmutableArray.Empty); } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index dc87255bb376b1..eaaa7d76ed94b5 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -550,7 +550,58 @@ public void MakeSimpleTypeNameThrowsForNonSimpleNames(string input) } else { - Assert.Throws(() => typeName.MakeSimpleTypeName()); + Assert.Throws(() => typeName.MakeSimpleTypeName(assemblyName: null)); + } + } + + [Theory] + [InlineData("Declaring+Nested")] + [InlineData("Declaring+Nested, Lib")] + [InlineData("Declaring+Deep+Deeper+Nested")] + [InlineData("Declaring+Deep+Deeper+Nested, Lib")] + public void MakeSimpleTypeName_NestedNames(string name) + { + AssemblyNameInfo assemblyName = new("New"); + TypeName parsed = TypeName.Parse(name.AsSpan()); + TypeName made = parsed.MakeSimpleTypeName(assemblyName); + + VerifyNestedNames(parsed, made, assemblyName); + } + + [Theory] + [InlineData(typeof(NestedNonGeneric_0.NestedGeneric_1))] + [InlineData(typeof(NestedGeneric_0))] + [InlineData(typeof(NestedGeneric_0.NestedNonGeneric_1))] + [InlineData(typeof(NestedGeneric_0.NestedGeneric_1))] + [InlineData(typeof(NestedGeneric_0.NestedGeneric_1.NestedNonGeneric_2))] + [InlineData(typeof(NestedGeneric_0.NestedGeneric_1.NestedGeneric_2))] + [InlineData(typeof(NestedGeneric_0.NestedGeneric_1.NestedGeneric_2.NestedNonGeneric_3))] + public void MakeGenericTypeName_NestedNames(Type type) + { + AssemblyNameInfo assemblyName = new("New"); + TypeName parsed = TypeName.Parse(type.AssemblyQualifiedName.AsSpan()); + TypeName made = parsed.GetGenericTypeDefinition().MakeSimpleTypeName(assemblyName).MakeGenericTypeName(parsed.GetGenericArguments()); + + VerifyNestedNames(parsed, made, assemblyName); + } + + private static void VerifyNestedNames(TypeName parsed, TypeName made, AssemblyNameInfo assemblyName) + { + while (true) + { + Assert.Equal(parsed.Name, made.Name); + Assert.Equal(parsed.FullName, made.FullName); + Assert.Equal(assemblyName, made.AssemblyName); + Assert.NotEqual(parsed.AssemblyQualifiedName, made.AssemblyQualifiedName); + Assert.EndsWith(assemblyName.FullName, made.AssemblyQualifiedName); + + if (!parsed.IsNested) + { + break; + } + + parsed = parsed.DeclaringType; + made = made.DeclaringType; } } @@ -809,6 +860,8 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) public class NestedNonGeneric_0 { public class NestedNonGeneric_1 { } + + public class NestedGeneric_1 { } } public class NestedGeneric_0 @@ -819,7 +872,10 @@ public class NestedGeneric_2 { public class NestedNonGeneric_3 { } } + + public class NestedNonGeneric_2 { } } + public class NestedNonGeneric_1 { } } } } From 19f1e82b57265656e8de7b3687ddb311052d77e2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 6 Aug 2024 16:26:14 +0200 Subject: [PATCH 11/18] update the code after reading it again on GH --- .../System.Reflection.Metadata/src/Resources/Strings.resx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx index 64264b093c0135..21bab756dd6a89 100644 --- a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx @@ -433,9 +433,6 @@ The given assembly name was invalid. - MakeGenericTypeName may only be called on a type name for which TypeName.IsSimple is true, for '{0}' it's not. - - - The value cannot be an empty string. + '{0}' is not a simple TypeName. \ No newline at end of file From 3abda021ce1e1afda4a080337fe4a57e6a25b171 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 12 Aug 2024 17:34:00 +0200 Subject: [PATCH 12/18] address code review feedback --- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 6 +++--- .../src/System/Reflection/Metadata/TypeName.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index 220ab322d7a311..d3c478d2769f97 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -155,9 +155,9 @@ internal TypeName GetArrayTypeName(ArrayInfo arrayInfo) }; // In general, arrayRank == 1 may have two different meanings: - // - [] is a single dimension and zero-indexed array (SZArray) - // - [*] is single dimension, custom offset array. - // Custom offset arrays are not supported by design, so in our case it's always SZArray. + // - [] is a single-dimensional array with a zero lower bound (SZArray), + // - [*] is a single-dimensional array with an arbitrary lower bound (variable bound array). + // Variable bound arrays are not supported by design, so in our case it's always SZArray. // That is why we don't call TypeName.MakeArrayTypeName(1) because it would create [*] instead of [] name. return arrayInfo.Rank == 1 ? elementTypeName.MakeSZArrayTypeName() diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 99dcc953d000a1..dbe363e05f0cd7 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -458,7 +458,7 @@ public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) /// /// rank is invalid. For example, 0 or negative. public TypeName MakeArrayTypeName(int rank) - => rank <= 0 || rank > 32 + => rank <= 0 ? throw new ArgumentOutOfRangeException(nameof(rank)) : MakeElementTypeName((sbyte)rank); From 50a58fb647a7d166cbc0cdaabd4e9d47e2b59190 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 12 Aug 2024 21:02:42 +0200 Subject: [PATCH 13/18] address code review feedback --- .../src/System/Formats/Nrbf/MemberTypeInfo.cs | 3 ++- .../src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs index d3c478d2769f97..9843a0b71f04c0 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberTypeInfo.cs @@ -151,7 +151,8 @@ internal TypeName GetArrayTypeName(ArrayInfo arrayInfo) BinaryType.Object => TypeNameHelpers.GetPrimitiveTypeName(TypeNameHelpers.ObjectPrimitiveType), BinaryType.ObjectArray => TypeNameHelpers.GetPrimitiveSZArrayTypeName(TypeNameHelpers.ObjectPrimitiveType), BinaryType.SystemClass => (TypeName)additionalInfo!, - _ => ((ClassTypeInfo)additionalInfo!).TypeName, + BinaryType.Class => ((ClassTypeInfo)additionalInfo!).TypeName, + _ => throw new ArgumentOutOfRangeException(paramName: nameof(binaryType), actualValue: binaryType, message: null) }; // In general, arrayRank == 1 may have two different meanings: diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index a2bf3774d360f5..a796c387d6020f 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -38,6 +38,7 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) PrimitiveType.Int32 => "System.Int32", PrimitiveType.UInt32 => "System.UInt32", PrimitiveType.Int64 => "System.Int64", + PrimitiveType.UInt64 => "System.UInt64", PrimitiveType.Single => "System.Single", PrimitiveType.Double => "System.Double", PrimitiveType.Decimal => "System.Decimal", @@ -47,7 +48,7 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) ObjectPrimitiveType => "System.Object", IntPtrPrimitiveType => "System.IntPtr", UIntPtrPrimitiveType => "System.UIntPtr", - _ => "System.UInt64", + _ => throw new ArgumentOutOfRangeException(paramName: nameof(primitiveType), actualValue: primitiveType, message: null) }; s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); @@ -82,12 +83,13 @@ internal static PrimitiveType GetPrimitiveType() else if (typeof(T) == typeof(decimal)) return PrimitiveType.Decimal; else if (typeof(T) == typeof(DateTime)) return PrimitiveType.DateTime; else if (typeof(T) == typeof(TimeSpan)) return PrimitiveType.TimeSpan; + else if (typeof(T) == typeof(string)) return PrimitiveType.String; else if (typeof(T) == typeof(IntPtr)) return IntPtrPrimitiveType; else if (typeof(T) == typeof(UIntPtr)) return UIntPtrPrimitiveType; else { - Debug.Assert(typeof(T) == typeof(string)); - return PrimitiveType.String; + Debug.Fail($"{typeof(T).Name} was not expected."); + throw new InvalidOperationException(); } } From ebfc35e545c5ca3bafd78bb6ce3cd327db3dc65c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Aug 2024 11:50:53 +0200 Subject: [PATCH 14/18] address code review feedback --- .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 26 ++++++++----------- .../System/Reflection/Metadata/TypeName.cs | 13 +++++----- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index a796c387d6020f..ff7d44868567df 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -16,15 +16,15 @@ internal static class TypeNameHelpers internal const PrimitiveType ObjectPrimitiveType = (PrimitiveType)19; internal const PrimitiveType IntPtrPrimitiveType = (PrimitiveType)20; internal const PrimitiveType UIntPtrPrimitiveType = (PrimitiveType)21; - private static readonly TypeName?[] s_PrimitiveTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; - private static readonly TypeName?[] s_PrimitiveSZArrayTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; - private static AssemblyNameInfo? s_CoreLibAssemblyName; + private static readonly TypeName?[] s_primitiveTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; + private static readonly TypeName?[] s_primitiveSZArrayTypeNames = new TypeName?[(int)UIntPtrPrimitiveType + 1]; + private static AssemblyNameInfo? s_coreLibAssemblyName; internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) { Debug.Assert(primitiveType is not (PrimitiveType.None or PrimitiveType.Null)); - TypeName? typeName = s_PrimitiveTypeNames[(int)primitiveType]; + TypeName? typeName = s_primitiveTypeNames[(int)primitiveType]; if (typeName is null) { string fullName = primitiveType switch @@ -51,17 +51,17 @@ internal static TypeName GetPrimitiveTypeName(PrimitiveType primitiveType) _ => throw new ArgumentOutOfRangeException(paramName: nameof(primitiveType), actualValue: primitiveType, message: null) }; - s_PrimitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); + s_primitiveTypeNames[(int)primitiveType] = typeName = TypeName.Parse(fullName.AsSpan()).WithCoreLibAssemblyName(); } return typeName; } internal static TypeName GetPrimitiveSZArrayTypeName(PrimitiveType primitiveType) { - TypeName? typeName = s_PrimitiveSZArrayTypeNames[(int)primitiveType]; + TypeName? typeName = s_primitiveSZArrayTypeNames[(int)primitiveType]; if (typeName is null) { - s_PrimitiveSZArrayTypeNames[(int)primitiveType] = typeName = GetPrimitiveTypeName(primitiveType).MakeSZArrayTypeName(); + s_primitiveSZArrayTypeNames[(int)primitiveType] = typeName = GetPrimitiveTypeName(primitiveType).MakeSZArrayTypeName(); } return typeName; } @@ -86,11 +86,7 @@ internal static PrimitiveType GetPrimitiveType() else if (typeof(T) == typeof(string)) return PrimitiveType.String; else if (typeof(T) == typeof(IntPtr)) return IntPtrPrimitiveType; else if (typeof(T) == typeof(UIntPtr)) return UIntPtrPrimitiveType; - else - { - Debug.Fail($"{typeof(T).Name} was not expected."); - throw new InvalidOperationException(); - } + else throw new InvalidOperationException(); } internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, BinaryLibraryRecord libraryRecord, PayloadOptions payloadOptions) @@ -104,7 +100,7 @@ internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, Debug.Assert(libraryRecord.RawLibraryName is not null); // Combining type and library allows us for handling truncated generic type names that may be present in resources. - ArraySegment assemblyQualifiedName = GetAssemblyQualifiedName(rawName, libraryRecord.RawLibraryName); + ArraySegment assemblyQualifiedName = RentAssemblyQualifiedName(rawName, libraryRecord.RawLibraryName); TypeName.TryParse(assemblyQualifiedName.AsSpan(), out TypeName? typeName, payloadOptions.TypeNameParseOptions); ArrayPool.Shared.Return(assemblyQualifiedName.Array!); @@ -130,7 +126,7 @@ internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadO .WithCoreLibAssemblyName(); // We know it's a System Record, so we set the LibraryName to CoreLib internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(s_CoreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); + => systemType.WithAssemblyName(s_coreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInfo assemblyName) { @@ -172,7 +168,7 @@ private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions return typeName; } - private static ArraySegment GetAssemblyQualifiedName(string typeName, string libraryName) + private static ArraySegment RentAssemblyQualifiedName(string typeName, string libraryName) { int length = typeName.Length + 1 + libraryName.Length; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index dbe363e05f0cd7..a1c15dfedc296a 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -430,15 +430,16 @@ public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) ? DeclaringType.MakeSimpleTypeName(assemblyName) : null; - return new TypeName(fullName: FullName, + return new TypeName(fullName: _fullName, assemblyName: assemblyName, elementOrGenericType: null, declaringType: declaringType, - genericTypeArguments: ImmutableArray.Empty); + genericTypeArguments: ImmutableArray.Empty, + nestedNameLength: _nestedNameLength); } /// - /// Returns a object representing a one-dimensional array + /// Creates a object representing a one-dimensional array /// of the current type, with a lower bound of zero. /// /// @@ -448,7 +449,7 @@ public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) public TypeName MakeSZArrayTypeName() => MakeElementTypeName(TypeNameParserHelpers.SZArray); /// - /// Returns a object representing an array of the current type, + /// Creates a object representing an array of the current type, /// with the specified number of dimensions. /// /// The number of dimensions for the array. This number must be more than zero and less than or equal to 32. @@ -463,7 +464,7 @@ public TypeName MakeArrayTypeName(int rank) : MakeElementTypeName((sbyte)rank); /// - /// Returns a object that represents a pointer to the current type. + /// Creates a object that represents a pointer to the current type. /// /// /// A object that represents a pointer to the current type. @@ -471,7 +472,7 @@ public TypeName MakeArrayTypeName(int rank) public TypeName MakePointerTypeName() => MakeElementTypeName(TypeNameParserHelpers.Pointer); /// - /// Returns a object that represents a managed reference to the current type. + /// Creates a object that represents a managed reference to the current type. /// /// /// A object that represents a managed reference to the current type. From f50ae19108ad11687a8b71313443cb7a14e5be06 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 13 Aug 2024 18:36:02 +0200 Subject: [PATCH 15/18] rename MakeSimpleTypeName to WithAssemblyName --- .../src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs | 12 ++++++------ .../ref/System.Reflection.Metadata.cs | 2 +- .../src/System/Reflection/Metadata/TypeName.cs | 4 ++-- .../tests/Metadata/TypeNameTests.cs | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index ff7d44868567df..eabac208332e47 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -93,7 +93,7 @@ internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, { if (libraryRecord.LibraryName is not null) { - return ParseWithoutAssemblyName(rawName, payloadOptions).WithAssemblyName(libraryRecord.LibraryName); + return ParseWithoutAssemblyName(rawName, payloadOptions).With(libraryRecord.LibraryName); } Debug.Assert(payloadOptions.UndoTruncatedTypeNames); @@ -126,15 +126,15 @@ internal static TypeName ParseSystemRecordTypeName(this string rawName, PayloadO .WithCoreLibAssemblyName(); // We know it's a System Record, so we set the LibraryName to CoreLib internal static TypeName WithCoreLibAssemblyName(this TypeName systemType) - => systemType.WithAssemblyName(s_coreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); + => systemType.With(s_coreLibAssemblyName ??= AssemblyNameInfo.Parse("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089".AsSpan())); - private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInfo assemblyName) + private static TypeName With(this TypeName typeName, AssemblyNameInfo assemblyName) { if (!typeName.IsSimple) { if (typeName.IsArray) { - TypeName newElementType = typeName.GetElementType().WithAssemblyName(assemblyName); + TypeName newElementType = typeName.GetElementType().With(assemblyName); return typeName.IsSZArray ? newElementType.MakeSZArrayTypeName() @@ -142,7 +142,7 @@ private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInf } else if (typeName.IsConstructedGenericType) { - TypeName newGenericTypeDefinition = typeName.GetGenericTypeDefinition().WithAssemblyName(assemblyName); + TypeName newGenericTypeDefinition = typeName.GetGenericTypeDefinition().With(assemblyName); // We don't change the assembly name of generic arguments on purpose. return newGenericTypeDefinition.MakeGenericTypeName(typeName.GetGenericArguments()); @@ -154,7 +154,7 @@ private static TypeName WithAssemblyName(this TypeName typeName, AssemblyNameInf } } - return typeName.MakeSimpleTypeName(assemblyName); + return typeName.WithAssemblyName(assemblyName); } private static TypeName ParseWithoutAssemblyName(string rawName, PayloadOptions payloadOptions) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index 3b2994c1aa6401..5a36e098782d4c 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2450,7 +2450,7 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName MakeByRefTypeName() { throw null; } public System.Reflection.Metadata.TypeName MakeGenericTypeName(System.Collections.Immutable.ImmutableArray typeArguments) { throw null; } public System.Reflection.Metadata.TypeName MakePointerTypeName() { throw null; } - public System.Reflection.Metadata.TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) { throw null; } + public System.Reflection.Metadata.TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) { throw null; } } public sealed partial class TypeNameParseOptions { diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index a1c15dfedc296a..ed7235609e572f 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -419,7 +419,7 @@ public int GetArrayRank() /// Assembly name. /// Created simple name. /// The current type name is not simple. - public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) + public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) { if (!IsSimple) { @@ -427,7 +427,7 @@ public TypeName MakeSimpleTypeName(AssemblyNameInfo? assemblyName) } TypeName? declaringType = IsNested - ? DeclaringType.MakeSimpleTypeName(assemblyName) + ? DeclaringType.WithAssemblyName(assemblyName) : null; return new TypeName(fullName: _fullName, diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index eaaa7d76ed94b5..1fbc9f4e011d17 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -528,14 +528,14 @@ public void MakeGenericTypeName() [InlineData("Declaring+Nested")] [InlineData("Declaring+Deep+Deeper+Nested")] [InlineData("Generic[[GenericArg]]")] - public void MakeSimpleTypeNameThrowsForNonSimpleNames(string input) + public void WithAssemblyNameThrowsForNonSimpleNames(string input) { TypeName typeName = TypeName.Parse(input.AsSpan()); if (typeName.IsSimple) { AssemblyNameInfo assemblyName = new("MyName"); - TypeName simple = typeName.MakeSimpleTypeName(assemblyName: assemblyName); + TypeName simple = typeName.WithAssemblyName(assemblyName: assemblyName); Assert.Equal(typeName.FullName, simple.FullName); // full name has not changed Assert.NotEqual(typeName.AssemblyQualifiedName, simple.AssemblyQualifiedName); @@ -550,7 +550,7 @@ public void MakeSimpleTypeNameThrowsForNonSimpleNames(string input) } else { - Assert.Throws(() => typeName.MakeSimpleTypeName(assemblyName: null)); + Assert.Throws(() => typeName.WithAssemblyName(assemblyName: null)); } } @@ -559,11 +559,11 @@ public void MakeSimpleTypeNameThrowsForNonSimpleNames(string input) [InlineData("Declaring+Nested, Lib")] [InlineData("Declaring+Deep+Deeper+Nested")] [InlineData("Declaring+Deep+Deeper+Nested, Lib")] - public void MakeSimpleTypeName_NestedNames(string name) + public void WithAssemblyName_NestedNames(string name) { AssemblyNameInfo assemblyName = new("New"); TypeName parsed = TypeName.Parse(name.AsSpan()); - TypeName made = parsed.MakeSimpleTypeName(assemblyName); + TypeName made = parsed.WithAssemblyName(assemblyName); VerifyNestedNames(parsed, made, assemblyName); } @@ -580,7 +580,7 @@ public void MakeGenericTypeName_NestedNames(Type type) { AssemblyNameInfo assemblyName = new("New"); TypeName parsed = TypeName.Parse(type.AssemblyQualifiedName.AsSpan()); - TypeName made = parsed.GetGenericTypeDefinition().MakeSimpleTypeName(assemblyName).MakeGenericTypeName(parsed.GetGenericArguments()); + TypeName made = parsed.GetGenericTypeDefinition().WithAssemblyName(assemblyName).MakeGenericTypeName(parsed.GetGenericArguments()); VerifyNestedNames(parsed, made, assemblyName); } From 69cd1fa095f144493e33ac88b8b1f7879f820718 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Aug 2024 09:39:48 +0200 Subject: [PATCH 16/18] Apply suggestions from code review Co-authored-by: Jeremy Barton Co-authored-by: Jan Kotas --- .../Formats/Nrbf/Utils/TypeNameHelpers.cs | 57 ++++++++++++------- .../System/Reflection/Metadata/TypeName.cs | 6 +- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs index eabac208332e47..97c3b4e42f68b7 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/TypeNameHelpers.cs @@ -68,25 +68,44 @@ internal static TypeName GetPrimitiveSZArrayTypeName(PrimitiveType primitiveType internal static PrimitiveType GetPrimitiveType() { - if (typeof(T) == typeof(bool)) return PrimitiveType.Boolean; - else if (typeof(T) == typeof(byte)) return PrimitiveType.Byte; - else if (typeof(T) == typeof(sbyte)) return PrimitiveType.SByte; - else if (typeof(T) == typeof(char)) return PrimitiveType.Char; - else if (typeof(T) == typeof(short)) return PrimitiveType.Int16; - else if (typeof(T) == typeof(ushort)) return PrimitiveType.UInt16; - else if (typeof(T) == typeof(int)) return PrimitiveType.Int32; - else if (typeof(T) == typeof(uint)) return PrimitiveType.UInt32; - else if (typeof(T) == typeof(long)) return PrimitiveType.Int64; - else if (typeof(T) == typeof(ulong)) return PrimitiveType.UInt64; - else if (typeof(T) == typeof(float)) return PrimitiveType.Single; - else if (typeof(T) == typeof(double)) return PrimitiveType.Double; - else if (typeof(T) == typeof(decimal)) return PrimitiveType.Decimal; - else if (typeof(T) == typeof(DateTime)) return PrimitiveType.DateTime; - else if (typeof(T) == typeof(TimeSpan)) return PrimitiveType.TimeSpan; - else if (typeof(T) == typeof(string)) return PrimitiveType.String; - else if (typeof(T) == typeof(IntPtr)) return IntPtrPrimitiveType; - else if (typeof(T) == typeof(UIntPtr)) return UIntPtrPrimitiveType; - else throw new InvalidOperationException(); + if (typeof(T) == typeof(bool)) + return PrimitiveType.Boolean; + else if (typeof(T) == typeof(byte)) + return PrimitiveType.Byte; + else if (typeof(T) == typeof(sbyte)) + return PrimitiveType.SByte; + else if (typeof(T) == typeof(char)) + return PrimitiveType.Char; + else if (typeof(T) == typeof(short)) + return PrimitiveType.Int16; + else if (typeof(T) == typeof(ushort)) + return PrimitiveType.UInt16; + else if (typeof(T) == typeof(int)) + return PrimitiveType.Int32; + else if (typeof(T) == typeof(uint)) + return PrimitiveType.UInt32; + else if (typeof(T) == typeof(long)) + return PrimitiveType.Int64; + else if (typeof(T) == typeof(ulong)) + return PrimitiveType.UInt64; + else if (typeof(T) == typeof(float)) + return PrimitiveType.Single; + else if (typeof(T) == typeof(double)) + return PrimitiveType.Double; + else if (typeof(T) == typeof(decimal)) + return PrimitiveType.Decimal; + else if (typeof(T) == typeof(DateTime)) + return PrimitiveType.DateTime; + else if (typeof(T) == typeof(TimeSpan)) + return PrimitiveType.TimeSpan; + else if (typeof(T) == typeof(string)) + return PrimitiveType.String; + else if (typeof(T) == typeof(IntPtr)) + return IntPtrPrimitiveType; + else if (typeof(T) == typeof(UIntPtr)) + return UIntPtrPrimitiveType; + else + throw new InvalidOperationException(); } internal static TypeName ParseNonSystemClassRecordTypeName(this string rawName, BinaryLibraryRecord libraryRecord, PayloadOptions payloadOptions) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index ed7235609e572f..a63c0bd325a32c 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -72,9 +72,9 @@ internal TypeName(string? fullName, #if SYSTEM_REFLECTION_METADATA private TypeName(string? fullName, AssemblyNameInfo? assemblyName, - TypeName? elementOrGenericType = default, - TypeName? declaringType = default, - ImmutableArray genericTypeArguments = default, + TypeName? elementOrGenericType, + TypeName? declaringType, + ImmutableArray genericTypeArguments, sbyte rankOrModifier = default, int nestedNameLength = -1) { From aa0870f585eb38b65317f2d22377ab01fcc096df Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Aug 2024 09:41:08 +0200 Subject: [PATCH 17/18] don't use sbyte to store array rank, the limit of 32 can be changed in the future and should not be enforced by the parser --- .../src/System/Reflection/Metadata/TypeName.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index a63c0bd325a32c..2331aa0425bbdb 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -25,7 +25,7 @@ sealed class TypeName /// Positive value is array rank. /// Negative value is modifier encoded using constants defined in . /// - private readonly sbyte _rankOrModifier; + private readonly int _rankOrModifier; /// /// To avoid the need of allocating a string for all declaring types (example: A+B+C+D+E+F+G), /// length of the name is stored and the fullName passed in ctor represents the full name of the nested type. @@ -50,7 +50,7 @@ internal TypeName(string? fullName, #else ImmutableArray.Builder? genericTypeArguments = default, #endif - sbyte rankOrModifier = default, + int rankOrModifier = default, int nestedNameLength = -1) { _fullName = fullName; @@ -75,7 +75,7 @@ private TypeName(string? fullName, TypeName? elementOrGenericType, TypeName? declaringType, ImmutableArray genericTypeArguments, - sbyte rankOrModifier = default, + int rankOrModifier = default, int nestedNameLength = -1) { _fullName = fullName; @@ -461,7 +461,7 @@ public TypeName WithAssemblyName(AssemblyNameInfo? assemblyName) public TypeName MakeArrayTypeName(int rank) => rank <= 0 ? throw new ArgumentOutOfRangeException(nameof(rank)) - : MakeElementTypeName((sbyte)rank); + : MakeElementTypeName(rank); /// /// Creates a object that represents a pointer to the current type. @@ -498,7 +498,7 @@ public TypeName MakeGenericTypeName(ImmutableArray typeArguments) return new TypeName(fullName: null, AssemblyName, elementOrGenericType: this, declaringType: _declaringType, genericTypeArguments: typeArguments); } - private TypeName MakeElementTypeName(sbyte rankOrModifier) + private TypeName MakeElementTypeName(int rankOrModifier) => new TypeName( fullName: null, assemblyName: AssemblyName, From 30be71e28757096140ef5b5fc86ce0e7e1044d0e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 14 Aug 2024 09:44:02 +0200 Subject: [PATCH 18/18] fix the build --- .../src/System/Reflection/Metadata/TypeName.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 2331aa0425bbdb..8920e5a31c39fb 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -503,6 +503,7 @@ private TypeName MakeElementTypeName(int rankOrModifier) fullName: null, assemblyName: AssemblyName, elementOrGenericType: this, + declaringType: null, genericTypeArguments: ImmutableArray.Empty, rankOrModifier: rankOrModifier); #endif