Skip to content

Commit 1b4a9c4

Browse files
Improve DisplayNameHelpers for NativeAOT (dotnet#70084)
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ý <MichalStrehovsky@users.noreply.github.com>
1 parent 655bc1d commit 1b4a9c4

11 files changed

Lines changed: 58 additions & 54 deletions

File tree

src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Reflection.Metadata;
56
using System.Text;
67

78
using Internal.TypeSystem;
9+
using Internal.TypeSystem.Ecma;
810

911
using Debug = System.Diagnostics.Debug;
1012

@@ -38,6 +40,12 @@ public static string GetDisplayName(this MethodDesc method)
3840
{
3941
sb.Append(method.OwningType.GetDisplayNameWithoutNamespace());
4042
}
43+
else if (method.GetPropertyForAccessor() is PropertyPseudoDesc property)
44+
{
45+
sb.Append(property.Name);
46+
sb.Append('.');
47+
sb.Append(property.GetMethod == method ? "get" : "set");
48+
}
4149
else
4250
{
4351
sb.Append(method.Name);
@@ -68,6 +76,20 @@ public static string GetDisplayName(this MethodDesc method)
6876
return sb.ToString();
6977
}
7078

79+
public static string GetParameterDisplayName(this EcmaMethod method, int parameterIndex)
80+
{
81+
var reader = method.MetadataReader;
82+
var methodDefinition = reader.GetMethodDefinition(method.Handle);
83+
foreach (var parameterHandle in methodDefinition.GetParameters())
84+
{
85+
var parameter = reader.GetParameter(parameterHandle);
86+
if (parameter.SequenceNumber == parameterIndex + 1)
87+
return reader.GetString(parameter.Name);
88+
}
89+
90+
return $"#{parameterIndex}";
91+
}
92+
7193
public static string GetDisplayName(this FieldDesc field)
7294
{
7395
return new StringBuilder(field.OwningType.GetDisplayName())

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MethodExtensions.cs renamed to src/coreclr/tools/Common/Compiler/MethodExtensions.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,25 @@ public static bool NotCallableWithoutOwningEEType(this MethodDesc method)
103103
(owningType is not MetadataType mdType || !mdType.IsModuleType) && /* Compiler parks some instance methods on the <Module> type */
104104
!method.IsSharedByGenericInstantiations; /* Current impl limitation; can be lifted */
105105
}
106+
107+
public static PropertyPseudoDesc GetPropertyForAccessor(this MethodDesc accessor)
108+
{
109+
if (accessor.GetTypicalMethodDefinition() is not EcmaMethod ecmaAccessor)
110+
return null;
111+
112+
var type = (EcmaType)ecmaAccessor.OwningType;
113+
var reader = type.MetadataReader;
114+
foreach (var propertyHandle in reader.GetTypeDefinition(type.Handle).GetProperties())
115+
{
116+
var accessors = reader.GetPropertyDefinition(propertyHandle).GetAccessors();
117+
if (ecmaAccessor.Handle == accessors.Getter
118+
|| ecmaAccessor.Handle == accessors.Setter)
119+
{
120+
return new PropertyPseudoDesc(type, propertyHandle);
121+
}
122+
}
123+
124+
return null;
125+
}
106126
}
107127
}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/PropertyPseudoDesc.cs renamed to src/coreclr/tools/Common/Compiler/PropertyPseudoDesc.cs

File renamed without changes.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,15 @@ internal static Origin GetMethodParameterFromIndex(MethodDesc method, int parame
3232

3333
internal static string GetParameterNameForErrorMessage(ParameterOrigin origin)
3434
{
35-
return $"#{origin.Index}";
35+
return GetParameterNameForErrorMessage(origin.Method, origin.Index);
36+
}
37+
38+
internal static string GetParameterNameForErrorMessage(MethodDesc method, int parameterIndex)
39+
{
40+
if (method.GetTypicalMethodDefinition() is EcmaMethod ecmaMethod)
41+
return ecmaMethod.GetParameterDisplayName(parameterIndex);
42+
43+
return $"#{parameterIndex}";
3644
}
3745

3846
internal static string GetMethodSignatureDisplayName(MethodDesc method)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -513,26 +513,4 @@ private static DefType[] TryGetExplicitlyImplementedInterfaces(this TypeDesc typ
513513
return Array.Empty<DefType>();
514514
}
515515
}
516-
517-
// Temporary local copy of the enum because the enum we're compiling against
518-
// doesn't define all the values. Can be removed once we update to .NET 6.
519-
public enum DynamicallyAccessedMemberTypes
520-
{
521-
None = 0,
522-
PublicParameterlessConstructor = 0x0001,
523-
PublicConstructors = 0x0002 | PublicParameterlessConstructor,
524-
NonPublicConstructors = 0x0004,
525-
PublicMethods = 0x0008,
526-
NonPublicMethods = 0x0010,
527-
PublicFields = 0x0020,
528-
NonPublicFields = 0x0040,
529-
PublicNestedTypes = 0x0080,
530-
NonPublicNestedTypes = 0x0100,
531-
PublicProperties = 0x0200,
532-
NonPublicProperties = 0x0400,
533-
PublicEvents = 0x0800,
534-
NonPublicEvents = 0x1000,
535-
Interfaces = 0x2000,
536-
All = ~None
537-
}
538516
}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/EcmaExtensions.cs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,25 +63,5 @@ public static PropertyPseudoDesc GetProperty(this MetadataType mdType, string na
6363

6464
return null;
6565
}
66-
67-
public static PropertyPseudoDesc GetPropertyForAccessor(this MethodDesc accessor)
68-
{
69-
if (accessor.GetTypicalMethodDefinition() is not EcmaMethod ecmaAccessor)
70-
return null;
71-
var type = (EcmaType)ecmaAccessor.OwningType;
72-
var reader = type.MetadataReader;
73-
var module = type.EcmaModule;
74-
foreach (var propertyHandle in reader.GetTypeDefinition(type.Handle).GetProperties())
75-
{
76-
var accessors = reader.GetPropertyDefinition(propertyHandle).GetAccessors();
77-
if (ecmaAccessor.Handle == accessors.Getter
78-
|| ecmaAccessor.Handle == accessors.Setter)
79-
{
80-
return new PropertyPseudoDesc(type, propertyHandle);
81-
}
82-
}
83-
84-
return null;
85-
}
8666
}
8767
}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/FlowAnnotations.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ protected override TypeAnnotations CreateValueFromKey(TypeDesc key)
371371

372372
if (!IsTypeInterestingForDataflow(signature[parameter.SequenceNumber - 1]))
373373
{
374-
_logger.LogWarning(method, DiagnosticId.DynamicallyAccessedMembersOnMethodParameterCanOnlyApplyToTypesOrStrings, $"#{parameter.SequenceNumber}", method.GetDisplayName());
374+
_logger.LogWarning(method, DiagnosticId.DynamicallyAccessedMembersOnMethodParameterCanOnlyApplyToTypesOrStrings, DiagnosticUtilities.GetParameterNameForErrorMessage(method, parameter.SequenceNumber - 1), method.GetDisplayName());
375375
continue;
376376
}
377377

@@ -714,8 +714,8 @@ void LogValidationWarning(object provider, object baseProvider, MethodDesc origi
714714
{
715715
case int parameterNumber:
716716
_logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMismatchOnMethodParameterBetweenOverrides,
717-
$"#{parameterNumber}", DiagnosticUtilities.GetMethodSignatureDisplayName(origin),
718-
$"#{parameterNumber}", DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider));
717+
DiagnosticUtilities.GetParameterNameForErrorMessage(origin, parameterNumber), DiagnosticUtilities.GetMethodSignatureDisplayName(origin),
718+
DiagnosticUtilities.GetParameterNameForErrorMessage((MethodDesc)baseProvider, parameterNumber), DiagnosticUtilities.GetMethodSignatureDisplayName((MethodDesc)baseProvider));
719719
break;
720720
case GenericParameterDesc genericParameterOverride:
721721
_logger.LogWarning(origin, DiagnosticId.DynamicallyAccessedMembersMismatchOnGenericParameterBetweenOverrides,

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DynamicDependencyAttributeAlgorithm.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.DependencyList;
1818
using MethodAttributes = System.Reflection.MethodAttributes;
19-
using DynamicallyAccessedMemberTypes = ILCompiler.Dataflow.DynamicallyAccessedMemberTypes;
2019

2120
namespace ILCompiler.DependencyAnalysis
2221
{

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,9 @@
290290
<Compile Include="..\..\Common\Compiler\InstructionSetSupport.cs" Link="Compiler\InstructionSetSupport.cs" />
291291
<Compile Include="..\..\Common\Compiler\Int128FieldLayoutAlgorithm.cs" Link="Compiler\Int128FieldLayoutAlgorithm.cs" />
292292
<Compile Include="..\..\Common\Compiler\InternalCompilerErrorException.cs" Link="Compiler\InternalCompilerErrorException.cs" />
293+
<Compile Include="..\..\Common\Compiler\MethodExtensions.cs" Link="Compiler\MethodExtensions.cs" />
293294
<Compile Include="..\..\Common\Compiler\NameMangler.cs" Link="Compiler\NameMangler.cs" />
295+
<Compile Include="..\..\Common\Compiler\PropertyPseudoDesc.cs" Link="Compiler\PropertyPseudoDesc.cs" />
294296
<Compile Include="..\..\Common\Compiler\SingleMethodRootProvider.cs" Link="Compiler\SingleMethodRootProvider.cs" />
295297
<Compile Include="..\..\Common\Compiler\TypeExtensions.cs" Link="Compiler\TypeExtensions.cs" />
296298
<Compile Include="..\..\Common\Compiler\VectorFieldLayoutAlgorithm.cs" Link="Compiler\VectorFieldLayoutAlgorithm.cs" />
@@ -506,15 +508,13 @@
506508
<Compile Include="Compiler\ManagedBinaryEmitter.cs" />
507509
<Compile Include="Compiler\MetadataManager.cs" />
508510
<Compile Include="Compiler\InteropStubManager.cs" />
509-
<Compile Include="Compiler\MethodExtensions.cs" />
510511
<Compile Include="Compiler\MultiFileCompilationModuleGroup.cs" />
511512
<Compile Include="Compiler\NativeLibraryInitializerRootProvider.cs" />
512513
<Compile Include="Compiler\NodeMangler.cs" />
513514
<Compile Include="Compiler\ObjectDumper.cs" />
514515
<Compile Include="Compiler\ExportsFileWriter.cs" />
515516
<Compile Include="Compiler\ProcessLinkerXmlBase.cs" />
516517
<Compile Include="Compiler\ProcessXmlBase.cs" />
517-
<Compile Include="Compiler\PropertyPseudoDesc.cs" />
518518
<Compile Include="Compiler\RootingHelpers.cs" />
519519
<Compile Include="Compiler\RootingServiceProvider.cs" />
520520
<Compile Include="Compiler\RuntimeConfigurationRootProvider.cs" />

src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,10 @@
9393
<Compile Include="..\..\Common\Compiler\InstructionSetSupport.cs" Link="Compiler\InstructionSetSupport.cs" />
9494
<Compile Include="..\..\Common\Compiler\Int128FieldLayoutAlgorithm.cs" Link="Compiler\Int128FieldLayoutAlgorithm.cs" />
9595
<Compile Include="..\..\Common\Compiler\InternalCompilerErrorException.cs" Link="Compiler\InternalCompilerErrorException.cs" />
96+
<Compile Include="..\..\Common\Compiler\MethodExtensions.cs" Link="Compiler\MethodExtensions.cs" />
9697
<Compile Include="..\..\Common\Compiler\NameMangler.cs" Link="Compiler\NameMangler.cs" />
9798
<Compile Include="..\..\Common\Compiler\SingleMethodRootProvider.cs" Link="Compiler\SingleMethodRootProvider.cs" />
99+
<Compile Include="..\..\Common\Compiler\PropertyPseudoDesc.cs" Link="Compiler\PropertyPseudoDesc.cs" />
98100
<Compile Include="..\..\Common\Compiler\TypeExtensions.cs" Link="Compiler\TypeExtensions.cs" />
99101
<Compile Include="..\..\Common\Compiler\VectorFieldLayoutAlgorithm.cs" Link="Compiler\VectorFieldLayoutAlgorithm.cs" />
100102
<Compile Include="..\..\Common\JitInterface\CorInfoTypes.VarInfo.cs" Link="JitInterface\CorInfoTypes.VarInfo.cs" />
@@ -278,6 +280,4 @@
278280
<Link>JitInterface\UnboxingMethodDesc.cs</Link>
279281
</Compile>
280282
</ItemGroup>
281-
282-
<Import Project="..\ILLink.Shared\ILLink.Shared.projitems" Label="Shared" />
283283
</Project>

0 commit comments

Comments
 (0)