From ed70b0a8e5c822ecf634523e42d7b29c1a4adefb Mon Sep 17 00:00:00 2001 From: jasperd Date: Fri, 5 Aug 2022 18:04:31 +0200 Subject: [PATCH 1/4] Emit parameter names Fixes #73051 --- .../StackTraceMetadata/MethodNameFormatter.cs | 57 +++++++++++++++---- .../tests/System/Environment.StackTrace.cs | 4 +- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs index ebfbab7a8e2c02..d3a9765ab57448 100644 --- a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs +++ b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs @@ -49,7 +49,6 @@ public static string FormatMethodName(MetadataReader metadataReader, TypeDefinit MethodNameFormatter formatter = new MethodNameFormatter(metadataReader, SigTypeContext.FromMethod(metadataReader, enclosingTypeHandle, methodHandle)); Method method = metadataReader.GetMethod(methodHandle); - MethodSignature methodSignature = metadataReader.GetMethodSignature(method.Signature); formatter.EmitTypeName(enclosingTypeHandle, namespaceQualified: true); formatter._outputBuilder.Append('.'); formatter.EmitString(method.Name); @@ -64,7 +63,7 @@ public static string FormatMethodName(MetadataReader metadataReader, TypeDefinit } else { - formatter._outputBuilder.Append(", "); + formatter._outputBuilder.Append(','); } formatter.EmitTypeName(handle, namespaceQualified: false); } @@ -73,7 +72,7 @@ public static string FormatMethodName(MetadataReader metadataReader, TypeDefinit formatter._outputBuilder.Append(']'); } - formatter.EmitMethodParameters(methodSignature); + formatter.EmitMethodParameters(methodHandle); return formatter._outputBuilder.ToString(); } @@ -124,28 +123,30 @@ private void EmitMethodReferenceName(MemberReferenceHandle memberRefHandle) private void EmitMethodInstantiationName(MethodInstantiationHandle methodInstHandle) { MethodInstantiation methodInst = _metadataReader.GetMethodInstantiation(methodInstHandle); - MethodSignature methodSignature; + if (methodInst.Method.HandleType == HandleType.MemberReference) { MemberReferenceHandle methodRefHandle = methodInst.Method.ToMemberReferenceHandle(_metadataReader); MemberReference methodRef = methodRefHandle.GetMemberReference(_metadataReader); - EmitContainingTypeAndMethodName(methodRef, out methodSignature); + EmitContainingTypeAndMethodName(methodRef, out MethodSignature methodSignature); + EmitGenericArguments(methodInst.GenericTypeArguments); + EmitMethodParameters(methodSignature); } else { QualifiedMethodHandle qualifiedMethodHandle = methodInst.Method.ToQualifiedMethodHandle(_metadataReader); QualifiedMethod qualifiedMethod = _metadataReader.GetQualifiedMethod(qualifiedMethodHandle); - EmitContainingTypeAndMethodName(qualifiedMethod, out methodSignature); + EmitContainingTypeAndMethodName(qualifiedMethod); + EmitGenericArguments(methodInst.GenericTypeArguments); + EmitMethodParameters(qualifiedMethod.Method); } - EmitGenericArguments(methodInst.GenericTypeArguments); - EmitMethodParameters(methodSignature); } private void EmitMethodDefinitionName(QualifiedMethodHandle qualifiedMethodHandle) { QualifiedMethod qualifiedMethod = _metadataReader.GetQualifiedMethod(qualifiedMethodHandle); - EmitContainingTypeAndMethodName(qualifiedMethod, out MethodSignature methodSignature); - EmitMethodParameters(methodSignature); + EmitContainingTypeAndMethodName(qualifiedMethod); + EmitMethodParameters(qualifiedMethod.Method); } /// @@ -161,10 +162,9 @@ private void EmitContainingTypeAndMethodName(MemberReference methodRef, out Meth EmitString(methodRef.Name); } - private void EmitContainingTypeAndMethodName(QualifiedMethod qualifiedMethod, out MethodSignature methodSignature) + private void EmitContainingTypeAndMethodName(QualifiedMethod qualifiedMethod) { Method method = _metadataReader.GetMethod(qualifiedMethod.Method); - methodSignature = _metadataReader.GetMethodSignature(method.Signature); EmitTypeName(qualifiedMethod.EnclosingType, namespaceQualified: true); _outputBuilder.Append('.'); EmitString(method.Name); @@ -181,6 +181,39 @@ private void EmitMethodParameters(MethodSignature methodSignature) _outputBuilder.Append(')'); } + /// + /// Emit parenthesized method argument type list with parameter names. + /// + /// Method handle to use for parameter formatting + private void EmitMethodParameters(MethodHandle methodHandle) + { + Method method = methodHandle.GetMethod(_metadataReader); + HandleCollection typeVector = method.Signature.GetMethodSignature(_metadataReader).Parameters; + ParameterHandleCollection.Enumerator names = method.Parameters.GetEnumerator(); + + bool first = true; + + _outputBuilder.Append('('); + foreach (Handle handle in typeVector) + { + if (first) + { + first = false; + } + else + { + _outputBuilder.Append(", "); + } + + names.MoveNext(); + + EmitTypeName(handle, namespaceQualified: false); + _outputBuilder.Append(' '); + EmitString(names.Current.GetParameter(_metadataReader).Name); + } + _outputBuilder.Append(')'); + } + /// /// Emit comma-separated list of type names into the output string builder. /// diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs index bca195864c8354..61a76d530377d4 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Runtime.CompilerServices; using System.Text; @@ -15,8 +16,9 @@ public class EnvironmentStackTrace static string s_stackTrace; [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/73051", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))] [ActiveIssue("https://github.com/mono/mono/issues/15315", TestRuntimes.Mono)] + // Retain parameter names for NativeAOT + [DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicMethods, typeof(EnvironmentStackTrace))] public void StackTraceTest() { //arrange From 1ad242771dd0fbac04387404ef974ef57886b0d7 Mon Sep 17 00:00:00 2001 From: jasperd Date: Fri, 12 Aug 2022 19:30:46 +0200 Subject: [PATCH 2/4] Address PR feedback --- .../StackTraceMetadata/MethodNameFormatter.cs | 45 +++++++++++++------ .../tests/System/Environment.StackTrace.cs | 3 +- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs index d3a9765ab57448..81a0140e7c8b9e 100644 --- a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs +++ b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; using System.Diagnostics; using System.Text; @@ -187,30 +185,49 @@ private void EmitMethodParameters(MethodSignature methodSignature) /// Method handle to use for parameter formatting private void EmitMethodParameters(MethodHandle methodHandle) { + bool EnumerateParameters(ref ParameterHandleCollection.Enumerator enumerator, out Parameter parameter) + { + bool hasNext = enumerator.MoveNext(); + parameter = hasNext ? enumerator.Current.GetParameter(_metadataReader) : default; + + return hasNext; + } + Method method = methodHandle.GetMethod(_metadataReader); HandleCollection typeVector = method.Signature.GetMethodSignature(_metadataReader).Parameters; - ParameterHandleCollection.Enumerator names = method.Parameters.GetEnumerator(); + ParameterHandleCollection.Enumerator parameters = method.Parameters.GetEnumerator(); - bool first = true; + bool hasNext = EnumerateParameters(ref parameters, out Parameter parameter); + if (hasNext && parameter.Sequence == 0) + { + hasNext = EnumerateParameters(ref parameters, out parameter); + } _outputBuilder.Append('('); - foreach (Handle handle in typeVector) + + uint typeIndex = 0; + foreach (Handle type in typeVector) { - if (first) - { - first = false; - } - else + if (typeIndex != 0) { _outputBuilder.Append(", "); } - names.MoveNext(); + EmitTypeName(type, namespaceQualified: false); - EmitTypeName(handle, namespaceQualified: false); - _outputBuilder.Append(' '); - EmitString(names.Current.GetParameter(_metadataReader).Name); + if (++typeIndex == parameter.Sequence && hasNext) + { + string name = parameter.Name.GetConstantStringValue(_metadataReader).Value; + hasNext = EnumerateParameters(ref parameters, out parameter); + + if (!string.IsNullOrEmpty(name)) + { + _outputBuilder.Append(' '); + _outputBuilder.Append(name); + } + } } + _outputBuilder.Append(')'); } diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs index 61a76d530377d4..a347cd8bae215a 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs @@ -17,8 +17,6 @@ public class EnvironmentStackTrace [Fact] [ActiveIssue("https://github.com/mono/mono/issues/15315", TestRuntimes.Mono)] - // Retain parameter names for NativeAOT - [DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicMethods, typeof(EnvironmentStackTrace))] public void StackTraceTest() { //arrange @@ -56,6 +54,7 @@ private void GenericFrame(T1 t1, T2 t2) } [MethodImpl(MethodImplOptions.NoInlining)] + [return: NotNull] // Add extra IL return parameter private static void StaticFrame(object obj) { s_stackTrace = Environment.StackTrace; From 88aa83c622bc8a3f493f3f3e4c6d364590901335 Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 16 Aug 2022 22:45:54 +0200 Subject: [PATCH 3/4] Rename things --- .../StackTraceMetadata/MethodNameFormatter.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs index 81a0140e7c8b9e..41226385ea86ac 100644 --- a/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs +++ b/src/coreclr/nativeaot/System.Private.StackTraceMetadata/src/Internal/StackTraceMetadata/MethodNameFormatter.cs @@ -185,11 +185,10 @@ private void EmitMethodParameters(MethodSignature methodSignature) /// Method handle to use for parameter formatting private void EmitMethodParameters(MethodHandle methodHandle) { - bool EnumerateParameters(ref ParameterHandleCollection.Enumerator enumerator, out Parameter parameter) + bool TryGetNextParameter(ref ParameterHandleCollection.Enumerator enumerator, out Parameter parameter) { bool hasNext = enumerator.MoveNext(); parameter = hasNext ? enumerator.Current.GetParameter(_metadataReader) : default; - return hasNext; } @@ -197,10 +196,10 @@ bool EnumerateParameters(ref ParameterHandleCollection.Enumerator enumerator, ou HandleCollection typeVector = method.Signature.GetMethodSignature(_metadataReader).Parameters; ParameterHandleCollection.Enumerator parameters = method.Parameters.GetEnumerator(); - bool hasNext = EnumerateParameters(ref parameters, out Parameter parameter); - if (hasNext && parameter.Sequence == 0) + bool hasParameter = TryGetNextParameter(ref parameters, out Parameter parameter); + if (hasParameter && parameter.Sequence == 0) { - hasNext = EnumerateParameters(ref parameters, out parameter); + hasParameter = TryGetNextParameter(ref parameters, out parameter); } _outputBuilder.Append('('); @@ -215,10 +214,10 @@ bool EnumerateParameters(ref ParameterHandleCollection.Enumerator enumerator, ou EmitTypeName(type, namespaceQualified: false); - if (++typeIndex == parameter.Sequence && hasNext) + if (++typeIndex == parameter.Sequence && hasParameter) { string name = parameter.Name.GetConstantStringValue(_metadataReader).Value; - hasNext = EnumerateParameters(ref parameters, out parameter); + hasParameter = TryGetNextParameter(ref parameters, out parameter); if (!string.IsNullOrEmpty(name)) { From 814a197c0114c5e4c78133cd8dd5976854c53c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 18 Aug 2022 13:50:32 +0900 Subject: [PATCH 4/4] Apply suggestions from code review --- .../tests/System/Environment.StackTrace.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs index a347cd8bae215a..255d03483505f2 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.StackTrace.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.IO; using System.Runtime.CompilerServices; using System.Text; @@ -54,7 +53,6 @@ private void GenericFrame(T1 t1, T2 t2) } [MethodImpl(MethodImplOptions.NoInlining)] - [return: NotNull] // Add extra IL return parameter private static void StaticFrame(object obj) { s_stackTrace = Environment.StackTrace;