Change S.R.M.TypeName to be consumed from System.Reflection.Metadata …

…package Contributes to dotnet#101541 Fixes dotnet#101628 Better unreachable code elimination in CoreLib
jkotas · Apr 28, 2024 · 5c6a76d · 5c6a76d
1 parent 6313ff8
commit 5c6a76d
Show file tree

Hide file tree

Showing 13 changed files with 95 additions and 120 deletions.
diff --git a/eng/Versions.props b/eng/Versions.props
@@ -119,7 +119,7 @@
     <SystemDrawingCommonVersion>8.0.0</SystemDrawingCommonVersion>
     <SystemIOFileSystemAccessControlVersion>5.0.0</SystemIOFileSystemAccessControlVersion>
     <SystemMemoryVersion>4.5.5</SystemMemoryVersion>
-    <SystemReflectionMetadataVersion>9.0.0-preview.4.24219.3</SystemReflectionMetadataVersion>
+    <SystemReflectionMetadataVersion>9.0.0-preview.4.24227.6</SystemReflectionMetadataVersion>
     <SystemReflectionMetadataLoadContextVersion>9.0.0-preview.4.24219.3</SystemReflectionMetadataLoadContextVersion>
     <SystemSecurityAccessControlVersion>6.0.0</SystemSecurityAccessControlVersion>
     <SystemSecurityCryptographyCngVersion>5.0.0</SystemSecurityCryptographyCngVersion>
@@ -210,7 +210,7 @@
     <!-- Mono Cecil -->
     <MicrosoftDotNetCecilVersion>0.11.4-alpha.24222.1</MicrosoftDotNetCecilVersion>
     <!-- ILCompiler -->
-    <MicrosoftDotNetILCompilerVersion>9.0.0-preview.4.24219.3</MicrosoftDotNetILCompilerVersion>
+    <MicrosoftDotNetILCompilerVersion>9.0.0-preview.4.24227.6</MicrosoftDotNetILCompilerVersion>
     <!-- ICU -->
     <MicrosoftNETCoreRuntimeICUTransportVersion>9.0.0-preview.5.24223.2</MicrosoftNETCoreRuntimeICUTransportVersion>
     <!-- MsQuic -->

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs
@@ -29,14 +29,16 @@ namespace System.Reflection
 {
     internal partial struct TypeNameParser
     {
+        private static readonly TypeNameParseOptions s_typeNameParseOptions = new() { MaxNodes = Int32.MaxValue };
+
         private ModuleDesc _module;
         private bool _throwIfNotFound;
         private Func<ModuleDesc, string, MetadataType> _canonResolver;
 
         public static TypeDesc ResolveType(ModuleDesc module, string name, bool throwIfNotFound,
             Func<ModuleDesc, string, MetadataType> canonResolver)
         {
-            if (!TypeName.TryParse(name.AsSpan(), out TypeName parsed))
+            if (!TypeName.TryParse(name.AsSpan(), out TypeName parsed, s_typeNameParseOptions))
             {
                 ThrowHelper.ThrowTypeLoadException(name, module);
             }

diff --git a/src/coreclr/tools/ILVerification/ILVerification.projitems b/src/coreclr/tools/ILVerification/ILVerification.projitems
@@ -66,30 +66,6 @@
     <Compile Include="$(ToolsCommonPath)TypeSystem\Common\Utilities\CustomAttributeTypeNameParser.cs">
       <Link>Utilities\CustomAttributeTypeNameParser.cs</Link>
     </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\HexConverter.cs">
-      <Link>Utilities\HexConverter.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\AssemblyNameFormatter.cs">
-      <Link>Utilities\AssemblyNameFormatter.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\AssemblyNameParser.cs">
-      <Link>Utilities\AssemblyNameParser.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\AssemblyNameInfo.cs">
-      <Link>Utilities\Metadata\AssemblyNameInfo.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeName.cs">
-      <Link>Utilities\TypeName.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParserOptions.cs">
-      <Link>Utilities\TypeNameParserOptions.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParser.cs">
-      <Link>Utilities\TypeNameParser.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParserHelpers.cs">
-      <Link>Utilities\TypeNameParserHelpers.cs</Link>
-    </Compile>
     <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
       <Link>System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs</Link>
     </Compile>

diff --git a/src/coreclr/tools/ILVerify/ILVerify.csproj b/src/coreclr/tools/ILVerify/ILVerify.csproj
@@ -17,6 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
     <PackageReference Include="System.CommandLine" Version="$(SystemCommandLineVersion)" />
   </ItemGroup>
 </Project>
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TypeNameParser.Dataflow.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TypeNameParser.Dataflow.cs
@@ -9,6 +9,8 @@ namespace System.Reflection
 {
     internal partial struct TypeNameParser
     {
+        private static readonly TypeNameParseOptions s_typeNameParseOptions = new() { MaxNodes = Int32.MaxValue };
+
         private TypeSystemContext _context;
         private ModuleDesc _callingModule;
         private List<ModuleDesc> _referencedModules;
@@ -17,7 +19,7 @@ internal partial struct TypeNameParser
         public static TypeDesc ResolveType(string name, ModuleDesc callingModule,
             TypeSystemContext context, List<ModuleDesc> referencedModules, out bool typeWasNotFoundInAssemblyNorBaseLibrary)
         {
-            if (!TypeName.TryParse(name, out TypeName parsed))
+            if (!TypeName.TryParse(name, out TypeName parsed, s_typeNameParseOptions))
             {
                 typeWasNotFoundInAssemblyNorBaseLibrary = false;
                 return null;

diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj
@@ -198,39 +198,12 @@
     <Compile Include="..\..\Common\TypeSystem\Common\Utilities\CustomAttributeTypeNameParser.cs">
       <Link>Utilities\CustomAttributeTypeNameParser.cs</Link>
     </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\HexConverter.cs">
-      <Link>Utilities\HexConverter.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\AssemblyNameFormatter.cs">
-      <Link>Utilities\AssemblyNameFormatter.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\AssemblyNameParser.cs">
-      <Link>Utilities\AssemblyNameParser.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\AssemblyNameInfo.cs">
-      <Link>Utilities\AssemblyNameInfo.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeName.cs">
-      <Link>Utilities\TypeName.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParser.cs">
-      <Link>Utilities\TypeNameParser.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParserHelpers.cs">
-      <Link>Utilities\TypeNameParserHelpers.cs</Link>
-    </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\Metadata\TypeNameParserOptions.cs">
-      <Link>Utilities\TypeNameParserOptions.cs</Link>
-    </Compile>
     <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Reflection\TypeNameParser.Helpers.cs">
       <Link>Utilities\CustomAttributeTypeNameParser.Helpers</Link>
     </Compile>
     <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Text\ValueStringBuilder.cs">
       <Link>Utilities\ValueStringBuilder.cs</Link>
     </Compile>
-    <Compile Include="$(LibrariesProjectRoot)\Common\src\System\Text\ValueStringBuilder.AppendSpanFormattable.cs">
-      <Link>Utilities\ValueStringBuilder.AppendSpanFormattable.cs</Link>
-    </Compile>
     <Compile Include="..\..\Common\TypeSystem\Common\Utilities\GCPointerMap.Algorithm.cs">
       <Link>Utilities\GCPointerMap.Algorithm.cs</Link>
     </Compile>
@@ -741,6 +714,10 @@
     <AdditionalFiles Include="BannedSymbols.txt" />
   </ItemGroup>
 
+  <ItemGroup>
+    <PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="$(MicrosoftCodeAnalysisBannedApiAnalyzersVersion)" Condition="'$(DotNetBuildSourceOnly)' != 'true'">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

diff --git a/src/libraries/Common/src/System/Reflection/Metadata/AssemblyNameInfo.cs b/src/libraries/Common/src/System/Reflection/Metadata/AssemblyNameInfo.cs
@@ -21,10 +21,10 @@ namespace System.Reflection.Metadata
     /// It's a more lightweight, immutable version of <seealso cref="AssemblyName"/> that does not pre-allocate <seealso cref="System.Globalization.CultureInfo"/> instances.
     /// </remarks>
     [DebuggerDisplay("{FullName}")]
-#if SYSTEM_PRIVATE_CORELIB
-    internal
-#else
+#if SYSTEM_REFLECTION_METADATA
     public
+#else
+    internal
 #endif
     sealed class AssemblyNameInfo
     {
@@ -182,11 +182,7 @@ public AssemblyName ToAssemblyName()
         public static AssemblyNameInfo Parse(ReadOnlySpan<char> assemblyName)
             => TryParse(assemblyName, out AssemblyNameInfo? result)
                 ? result!
-#if SYSTEM_REFLECTION_METADATA || SYSTEM_PRIVATE_CORELIB
                 : throw new ArgumentException(SR.InvalidAssemblyName, nameof(assemblyName));
-#else // tools that reference this file as a link
-                : throw new ArgumentException("The given assembly name was invalid.", nameof(assemblyName));
-#endif
 
         /// <summary>
         /// Tries to parse a span of characters into an assembly name.

diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeName.cs
@@ -16,10 +16,10 @@
 namespace System.Reflection.Metadata
 {
     [DebuggerDisplay("{AssemblyQualifiedName}")]
-#if SYSTEM_PRIVATE_CORELIB
-    internal
-#else
+#if SYSTEM_REFLECTION_METADATA
     public
+#else
+    internal
 #endif
     sealed class TypeName
     {

diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParser.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParser.cs
@@ -17,7 +17,8 @@ namespace System.Reflection.Metadata
     [DebuggerDisplay("{_inputString}")]
     internal ref struct TypeNameParser
     {
-        private static readonly TypeNameParseOptions _defaults = new();
+        private static readonly TypeNameParseOptions s_defaults = new();
+
         private readonly bool _throwOnError;
         private readonly TypeNameParseOptions _parseOptions;
         private ReadOnlySpan<char> _inputString;
@@ -26,7 +27,7 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
         {
             _inputString = name;
             _throwOnError = throwOnError;
-            _parseOptions = options ?? _defaults;
+            _parseOptions = options ?? s_defaults;
         }
 
         internal static TypeName? Parse(ReadOnlySpan<char> typeName, bool throwOnError, TypeNameParseOptions? options = default)
@@ -50,7 +51,7 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
             {
                 if (throwOnError)
                 {
-                    if (recursiveDepth >= parser._parseOptions.MaxNodes)
+                    if (parser._parseOptions.IsMaxDepthExceeded(recursiveDepth))
                     {
                         ThrowInvalidOperation_MaxNodesExceeded(parser._parseOptions.MaxNodes);
                     }
@@ -249,7 +250,7 @@ private bool TryParseAssemblyName(ref AssemblyNameInfo? assemblyName)
 
         private bool TryDive(ref int depth)
         {
-            if (depth >= _parseOptions.MaxNodes)
+            if (_parseOptions.IsMaxDepthExceeded(depth))
             {
                 return false;
             }

diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParserHelpers.cs
@@ -331,53 +331,64 @@ internal static bool TryStripFirstCharAndTrailingSpaces(ref ReadOnlySpan<char> s
         }
 
         [DoesNotReturn]
-        internal static void ThrowInvalidOperation_MaxNodesExceeded(int limit) => throw
-#if SYSTEM_REFLECTION_METADATA
-            new InvalidOperationException(SR.Format(SR.InvalidOperation_MaxNodesExceeded, limit));
-#else // corelib and tools that reference this file as a link
-            new InvalidOperationException();
-#endif
+        internal static void ThrowArgumentException_InvalidTypeName(int errorIndex)
+        {
+            throw new ArgumentException(SR.Argument_InvalidTypeName, $"typeName@{errorIndex}");
+        }
 
         [DoesNotReturn]
-        internal static void ThrowArgumentException_InvalidTypeName(int errorIndex) => throw
-#if SYSTEM_PRIVATE_CORELIB
-            new ArgumentException(SR.Arg_ArgumentException, $"typeName@{errorIndex}");
-#elif SYSTEM_REFLECTION_METADATA
-            new ArgumentException(SR.Argument_InvalidTypeName, $"typeName@{errorIndex}");
-#else // tools that reference this file as a link
-            new ArgumentException();
+        internal static void ThrowInvalidOperation_MaxNodesExceeded(int limit)
+        {
+#if SYSTEM_REFLECTION_METADATA
+            throw new InvalidOperationException(SR.Format(SR.InvalidOperation_MaxNodesExceeded, limit));
+#else
+            Debug.Fail("Expected to be unreachable");
+            throw new InvalidOperationException();
 #endif
+        }
 
         [DoesNotReturn]
-        internal static void ThrowInvalidOperation_NotGenericType() => throw
-#if SYSTEM_REFLECTION_METADATA || SYSTEM_PRIVATE_CORELIB
-            new InvalidOperationException(SR.InvalidOperation_NotGenericType);
-#else // tools that reference this file as a link
-            new InvalidOperationException();
+        internal static void ThrowInvalidOperation_NotGenericType()
+        {
+#if SYSTEM_REFLECTION_METADATA
+            throw new InvalidOperationException(SR.InvalidOperation_NotGenericType);
+#else
+            Debug.Fail("Expected to be unreachable");
+            throw new InvalidOperationException();
 #endif
+        }
 
         [DoesNotReturn]
-        internal static void ThrowInvalidOperation_NotNestedType() => throw
+        internal static void ThrowInvalidOperation_NotNestedType()
+        {
 #if SYSTEM_REFLECTION_METADATA
-            new InvalidOperationException(SR.InvalidOperation_NotNestedType);
-#else // corelib and tools that reference this file as a link
-            new InvalidOperationException();
+            throw new InvalidOperationException(SR.InvalidOperation_NotNestedType);
+#else
+            Debug.Fail("Expected to be unreachable");
+            throw new InvalidOperationException();
 #endif
+        }
 
         [DoesNotReturn]
-        internal static void ThrowInvalidOperation_NoElement() => throw
+        internal static void ThrowInvalidOperation_NoElement()
+        {
 #if SYSTEM_REFLECTION_METADATA
-            new InvalidOperationException(SR.InvalidOperation_NoElement);
-#else // corelib and tools that reference this file as a link
-            new InvalidOperationException();
+            throw new InvalidOperationException(SR.InvalidOperation_NoElement);
+#else
+            Debug.Fail("Expected to be unreachable");
+            throw new InvalidOperationException();
 #endif
+        }
 
         [DoesNotReturn]
-        internal static void ThrowInvalidOperation_HasToBeArrayClass() => throw
-#if SYSTEM_REFLECTION_METADATA || SYSTEM_PRIVATE_CORELIB
-            new InvalidOperationException(SR.Argument_HasToBeArrayClass);
-#else // tools that reference this file as a link
-            new InvalidOperationException();
+        internal static void ThrowInvalidOperation_HasToBeArrayClass()
+        {
+#if SYSTEM_REFLECTION_METADATA
+            throw new InvalidOperationException(SR.Argument_HasToBeArrayClass);
+#else
+            Debug.Fail("Expected to be unreachable");
+            throw new InvalidOperationException();
 #endif
+        }
     }
 }
diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParserOptions.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameParserOptions.cs
@@ -3,19 +3,9 @@
 
 namespace System.Reflection.Metadata
 {
-#if SYSTEM_PRIVATE_CORELIB
-    internal
-#else
-    public
-#endif
-    sealed class TypeNameParseOptions
+    public sealed class TypeNameParseOptions
     {
-        private int _maxNodes =
-#if SYSTEM_PRIVATE_CORELIB
-            int.MaxValue; // CoreLib has never introduced any limits
-#else
-            20;
-#endif
+        private int _maxNodes = 20;
 
         /// <summary>
         /// Limits the maximum value of <seealso cref="TypeName.GetNodeCount">node count</seealso> that parser can handle.
@@ -37,5 +27,7 @@ public int MaxNodes
                 _maxNodes = value;
             }
         }
+
+        internal bool IsMaxDepthExceeded(int depth) => depth >= _maxNodes;
     }
 }
diff --git a/src/libraries/Common/src/System/Reflection/TypeNameParser.Helpers.cs b/src/libraries/Common/src/System/Reflection/TypeNameParser.Helpers.cs
@@ -8,6 +8,28 @@
 
 #nullable enable
 
+#if SYSTEM_PRIVATE_CORELIB
+namespace System.Reflection.Metadata
+{
+    internal struct TypeNameParseOptions
+    {
+        public TypeNameParseOptions() { }
+#pragma warning disable CA1822 // Mark members as static
+        // CoreLib does not enforce any limits
+        public bool IsMaxDepthExceeded(int _) => false;
+        public int MaxNodes
+        {
+            get
+            {
+                 Debug.Fail("Expected to be unreachable");
+                 return 0;
+            }
+        }
+#pragma warning restore CA1822
+    }
+}
+#endif
+
 namespace System.Reflection
 {
     internal partial struct TypeNameParser