From 1b4a9c47ba8e4ad53562e003240527b738b0e7d1 Mon Sep 17 00:00:00 2001 From: Vitek Karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 2 Jun 2022 07:22:06 -0700 Subject: [PATCH] Improve DisplayNameHelpers for NativeAOT (#70084) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These helpers are used to report names of things in warnings. The functional changes are: * For method parameters, use the parameter name if available (and only if not fallback to the #1 notation) * For property accessor methods, use the C# naming scheme, so for example Type.Property.get instead of Type.get_Property. Both of these changes are in preparation to bring NativeAOT closer in behavior to ILLink and the trim analyzers. For this I moved some of the helpers to the common shared code. Some unrelated code cleanup as well. Co-authored-by: Michal Strehovský --- .../Common/Compiler/DisplayNameHelpers.cs | 22 +++++++++++++++++++ .../Compiler/MethodExtensions.cs | 20 +++++++++++++++++ .../Compiler/PropertyPseudoDesc.cs | 0 .../Compiler/Dataflow/DiagnosticUtilities.cs | 10 ++++++++- .../DynamicallyAccessedMembersBinder.cs | 22 ------------------- .../Compiler/Dataflow/EcmaExtensions.cs | 20 ----------------- .../Compiler/Dataflow/FlowAnnotations.cs | 6 ++--- .../DynamicDependencyAttributeAlgorithm.cs | 1 - .../ILCompiler.Compiler.csproj | 4 ++-- .../ILCompiler.ReadyToRun.csproj | 4 ++-- src/coreclr/tools/aot/crossgen2.sln | 3 --- 11 files changed, 58 insertions(+), 54 deletions(-) rename src/coreclr/tools/{aot/ILCompiler.Compiler => Common}/Compiler/MethodExtensions.cs (84%) rename src/coreclr/tools/{aot/ILCompiler.Compiler => Common}/Compiler/PropertyPseudoDesc.cs (100%) diff --git a/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs b/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs index 0ee508d160c65..0c4b0e1684473 100644 --- a/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs +++ b/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Reflection.Metadata; using System.Text; using Internal.TypeSystem; +using Internal.TypeSystem.Ecma; using Debug = System.Diagnostics.Debug; @@ -38,6 +40,12 @@ public static string GetDisplayName(this MethodDesc method) { sb.Append(method.OwningType.GetDisplayNameWithoutNamespace()); } + else if (method.GetPropertyForAccessor() is PropertyPseudoDesc property) + { + sb.Append(property.Name); + sb.Append('.'); + sb.Append(property.GetMethod == method ? "get" : "set"); + } else { sb.Append(method.Name); @@ -68,6 +76,20 @@ public static string GetDisplayName(this MethodDesc method) return sb.ToString(); } + public static string GetParameterDisplayName(this EcmaMethod method, int parameterIndex) + { + var reader = method.MetadataReader; + var methodDefinition = reader.GetMethodDefinition(method.Handle); + foreach (var parameterHandle in methodDefinition.GetParameters()) + { + var parameter = reader.GetParameter(parameterHandle); + if (parameter.SequenceNumber == parameterIndex + 1) + return reader.GetString(parameter.Name); + } + + return $"#{parameterIndex}"; + } + public static string GetDisplayName(this FieldDesc field) { return new StringBuilder(field.OwningType.GetDisplayName()) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MethodExtensions.cs b/src/coreclr/tools/Common/Compiler/MethodExtensions.cs similarity index 84% rename from src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MethodExtensions.cs rename to src/coreclr/tools/Common/Compiler/MethodExtensions.cs index b7aeee454ef02..af23ea53fb811 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MethodExtensions.cs +++ b/src/coreclr/tools/Common/Compiler/MethodExtensions.cs @@ -103,5 +103,25 @@ public static bool NotCallableWithoutOwningEEType(this MethodDesc method) (owningType is not MetadataType mdType || !mdType.IsModuleType) && /* Compiler parks some instance methods on the type */ !method.IsSharedByGenericInstantiations; /* Current impl limitation; can be lifted */ } + + public static PropertyPseudoDesc GetPropertyForAccessor(this MethodDesc accessor) + { + if (accessor.GetTypicalMethodDefinition() is not EcmaMethod ecmaAccessor) + return null; + + var type = (EcmaType)ecmaAccessor.OwningType; + var reader = type.MetadataReader; + foreach (var propertyHandle in reader.GetTypeDefinition(type.Handle).GetProperties()) + { + var accessors = reader.GetPropertyDefinition(propertyHandle).GetAccessors(); + if (ecmaAccessor.Handle == accessors.Getter + || ecmaAccessor.Handle == accessors.Setter) + { + return new PropertyPseudoDesc(type, propertyHandle); + } + } + + return null; + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/PropertyPseudoDesc.cs b/src/coreclr/tools/Common/Compiler/PropertyPseudoDesc.cs similarity index 100% rename from src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/PropertyPseudoDesc.cs rename to src/coreclr/tools/Common/Compiler/PropertyPseudoDesc.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs index fe2e79caec050..828c073f24687 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs @@ -32,7 +32,15 @@ internal static Origin GetMethodParameterFromIndex(MethodDesc method, int parame internal static string GetParameterNameForErrorMessage(ParameterOrigin origin) { - return $"#{origin.Index}"; + return GetParameterNameForErrorMessage(origin.Method, origin.Index); + } + + internal static string GetParameterNameForErrorMessage(MethodDesc method, int parameterIndex) + { + if (method.GetTypicalMethodDefinition() is EcmaMethod ecmaMethod) + return ecmaMethod.GetParameterDisplayName(parameterIndex); + + return $"#{parameterIndex}"; } internal static string GetMethodSignatureDisplayName(MethodDesc method) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs index 9eabda7ff273c..1704c3c826794 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs @@ -513,26 +513,4 @@ private static DefType[] TryGetExplicitlyImplementedInterfaces(this TypeDesc typ return Array.Empty(); } } - - // Temporary local copy of the enum because the enum we're compiling against - // doesn't define all the values. Can be removed once we update to .NET 6. - public enum DynamicallyAccessedMemberTypes - { - None = 0, - PublicParameterlessConstructor = 0x0001, - PublicConstructors = 0x0002 | PublicParameterlessConstructor, - NonPublicConstructors = 0x0004, - PublicMethods = 0x0008, - NonPublicMethods = 0x0010, - PublicFields = 0x0020, - NonPublicFields = 0x0040, - PublicNestedTypes = 0x0080, - NonPublicNestedTypes = 0x0100, - PublicProperties = 0x0200, - NonPublicProperties = 0x0400, - PublicEvents = 0x0800, - NonPublicEvents = 0x1000, - Interfaces = 0x2000, - All = ~None - } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/EcmaExtensions.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/EcmaExtensions.cs index a617257dde715..07dcfb8362cdb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/EcmaExtensions.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/EcmaExtensions.cs @@ -63,25 +63,5 @@ public static PropertyPseudoDesc GetProperty(this MetadataType mdType, string na return null; } - - public static PropertyPseudoDesc GetPropertyForAccessor(this MethodDesc accessor) - { - if (accessor.GetTypicalMethodDefinition() is not EcmaMethod ecmaAccessor) - return null; - var type = (EcmaType)ecmaAccessor.OwningType; - var reader = type.MetadataReader; - var module = type.EcmaModule; - foreach (var propertyHandle in reader.GetTypeDefinition(type.Handle).GetProperties()) - { - var accessors = reader.GetPropertyDefinition(propertyHandle).GetAccessors(); - if (ecmaAccessor.Handle == accessors.Getter - || ecmaAccessor.Handle == accessors.Setter) - { - return new PropertyPseudoDesc(type, propertyHandle); - } - } - - return null; - } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs index 6e659a95eb7e7..107a15ea48199 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs @@ -371,7 +371,7 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key) if (!IsTypeInterestingForDataflow(signature[parameter.SequenceNumber - 1])) { - _logger.LogWarning(method, DiagnosticId.DynamicallyAccessedMembersOnMethodParameterCanOnlyApplyToTypesOrStrings, $"#{parameter.SequenceNumber}", method.GetDisplayName()); + _logger.LogWarning(method, DiagnosticId.DynamicallyAccessedMembersOnMethodParameterCanOnlyApplyToTypesOrStrings, DiagnosticUtilities.GetParameterNameForErrorMessage(method, parameter.SequenceNumber - 1), method.GetDisplayName()); continue; } @@ -714,8 +714,8 @@ void LogValidationWarning(object provider, object baseProvider, MethodDesc origi { case int parameterNumber: _logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodParameterBetweenOverrides, - $"#{parameterNumber}", DiagnosticUtilities.GetMethodSignatureDisplayName(origin), - $"#{parameterNumber}", DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider)); + DiagnosticUtilities.GetParameterNameForErrorMessage(origin, parameterNumber), DiagnosticUtilities.GetMethodSignatureDisplayName(origin), + DiagnosticUtilities.GetParameterNameForErrorMessage((MethodDesc)baseProvider, parameterNumber), DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider)); break; case GenericParameterDesc genericParameterOverride: _logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMismatchOnGenericParameterBetweenOverrides, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DynamicDependencyAttributeAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DynamicDependencyAttributeAlgorithm.cs index a6bca843c6b08..4e3afab67672f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DynamicDependencyAttributeAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DynamicDependencyAttributeAlgorithm.cs @@ -16,7 +16,6 @@ using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore.DependencyList; using MethodAttributes = System.Reflection.MethodAttributes; -using DynamicallyAccessedMemberTypes = ILCompiler.Dataflow.DynamicallyAccessedMemberTypes; namespace ILCompiler.DependencyAnalysis { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index fc49eb64df0df..754a420618262 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -290,7 +290,9 @@ + + @@ -506,7 +508,6 @@ - @@ -514,7 +515,6 @@ - diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index b28729ea78291..289f60294a938 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -93,8 +93,10 @@ + + @@ -278,6 +280,4 @@ JitInterface\UnboxingMethodDesc.cs - - diff --git a/src/coreclr/tools/aot/crossgen2.sln b/src/coreclr/tools/aot/crossgen2.sln index c1054e8c3ccf2..ee4d3a0973f01 100644 --- a/src/coreclr/tools/aot/crossgen2.sln +++ b/src/coreclr/tools/aot/crossgen2.sln @@ -17,9 +17,6 @@ EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ILCompiler.TypeSystem.Tests", "ILCompiler.TypeSystem.Tests\ILCompiler.TypeSystem.Tests.csproj", "{9E65EC58-B500-4C4A-B57D-BF242129A3C6}" EndProject Global - GlobalSection(SharedMSBuildProjectFiles) = preSolution - ILLink.Shared\ILLink.Shared.projitems*{83a832de-bf4a-44c4-b361-90f5f88b979b}*SharedItemsImports = 5 - EndGlobalSection GlobalSection(SolutionConfigurationPlatforms) = preSolution Checked|Any CPU = Checked|Any CPU Checked|x64 = Checked|x64