Skip to content

Find all generic interface impls #103661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed

Find all generic interface impls #103661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d6564fb
Use recursive-interface-aware types and methods in TypeMapInfo
jtschuster Jun 4, 2024
8e94b0e
Use RuntimeInterfaceImplementations in MethodBodyScanner and ICustomM…
jtschuster Jun 11, 2024
cf304f2
Add remarks regarding checking for marked ifaceImpls with RuntimeInte…
jtschuster Jun 11, 2024
fcbfbb7
Fix ApiCompat failures
jtschuster Jun 11, 2024
e5e0679
Fix visibility and sealed warnings
jtschuster Jun 11, 2024
451b677
Add DAM annotations for recursive interfaces
jtschuster Jun 12, 2024
1e1451a
Add additional test cases and annotate reflection types with ifdefs
jtschuster Jun 13, 2024
ac5a42a
Use suppressions instead of annotating in System.Reflection.Context
jtschuster Jun 13, 2024
4e1f6f9
Add test and fix for failure in System.Text.Json
jtschuster Jun 17, 2024
f8dad6b
Check for implementor to be marked, not overridingMethod.DeclaringType
jtschuster Jun 17, 2024
2aca6c4
PR feedback:
jtschuster Jun 17, 2024
bb308c7
Separate out generic DIM matching fix
jtschuster Jun 18, 2024
fd1d44f
Revert "Separate out generic DIM matching fix"
jtschuster Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion src/libraries/System.Reflection.Context/src/ILLink/ILLink.Suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,5 +523,53 @@
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Delegation.DelegatingType.GetInterface(System.String,System.Boolean)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetField(System.String,System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetFields(System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetMembers(System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetMethods(System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetProperties(System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Projection.ProjectingType.GetDefaultMembers()</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Custom.CustomType.GetProperties(System.Reflection.BindingFlags)</property>
</attribute>
<attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<argument>ILLink</argument>
<argument>IL2094</argument>
<property name="Scope">member</property>
<property name="Target">M:System.Reflection.Context.Custom.CustomType.GetMethods(System.Reflection.BindingFlags)</property>
</attribute>
</assembly>
</linker>
</linker>
89 changes: 41 additions & 48 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand Down Expand Up @@ -738,18 +739,19 @@ internal static bool IsInterfaceImplementationMarkedRecursively (TypeDefinition
return true;
}
}

if (type.BaseType is not null && context.Resolve (type.BaseType) is { } baseType)
return IsInterfaceImplementationMarkedRecursively (baseType, interfaceType, context);
return false;
}

void ProcessDefaultImplementation (OverrideInformation ov, MessageOrigin origin)
{
Debug.Assert (ov.IsOverrideOfInterfaceMember);
if ((!ov.Override.IsStatic && !Annotations.IsInstantiated (ov.InterfaceImplementor.Implementor))
|| ov.Override.IsStatic && !Annotations.IsRelevantToVariantCasting (ov.InterfaceImplementor.Implementor))
if ((!ov.Override.IsStatic && !Annotations.IsInstantiated (ov.RuntimeInterfaceImplementation.Implementor))
|| ov.Override.IsStatic && !Annotations.IsRelevantToVariantCasting (ov.RuntimeInterfaceImplementation.Implementor))
return;

MarkInterfaceImplementation (ov.InterfaceImplementor.InterfaceImplementation, origin);
MarkRuntimeInterfaceImplementation (ov.RuntimeInterfaceImplementation, origin);
}

void MarkMarshalSpec (IMarshalInfoProvider spec, in DependencyInfo reason, MessageOrigin origin)
Expand Down Expand Up @@ -2398,35 +2400,37 @@ void MarkNamedProperty (TypeDefinition type, string property_name, in Dependency

void MarkInterfaceImplementations (TypeDefinition type)
{
var ifaces = Annotations.GetRecursiveInterfaces (type);
var ifaces = Annotations.GetRuntimeInterfaces (type);
if (ifaces is null)
return;
foreach (var (ifaceType, impls) in ifaces) {
foreach (var runtimeInterface in ifaces) {
// Only mark interface implementations of interface types that have been marked.
// This enables stripping of interfaces that are never used
if (ShouldMarkInterfaceImplementationList (type, impls, ifaceType))
MarkInterfaceImplementationList (impls, new MessageOrigin (type));
if (ShouldMarkRuntimeInterfaceImplementation (runtimeInterface))
MarkRuntimeInterfaceImplementation (runtimeInterface, new MessageOrigin (type));
}
}


protected virtual bool ShouldMarkInterfaceImplementationList (TypeDefinition type, List<InterfaceImplementation> ifaces, TypeReference ifaceType)
internal bool ShouldMarkRuntimeInterfaceImplementation (RuntimeInterfaceImplementation runtimeInterfaceImplementation)
{
var type = runtimeInterfaceImplementation.Implementor;
var ifaces = runtimeInterfaceImplementation.InterfaceImplementation;
var ifaceType = runtimeInterfaceImplementation.InterfaceTypeDefinition;
if (ifaces.All (Annotations.IsMarked))
return false;

if (!Context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, type))
return true;

if (Context.Resolve (ifaceType) is not TypeDefinition resolvedInterfaceType)
if (ifaceType is null)
return false;

if (Annotations.IsMarked (resolvedInterfaceType))
if (Annotations.IsMarked (ifaceType))
return true;

// It's hard to know if a com or windows runtime interface will be needed from managed code alone,
// so as a precaution we will mark these interfaces once the type is instantiated
if (Context.KeepComInterfaces && (resolvedInterfaceType.IsImport || resolvedInterfaceType.IsWindowsRuntime))
if (Context.KeepComInterfaces && (ifaceType.IsImport || ifaceType.IsWindowsRuntime))
return true;

return IsFullyPreserved (type);
Expand Down Expand Up @@ -2509,11 +2513,13 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat
if (Annotations.IsMarked (method))
return false;

if (!Annotations.IsMarked (overrideInformation.RuntimeInterfaceImplementation.Implementor))
return false;

// If the interface implementation is not marked, do not mark the implementation method
// A type that doesn't implement the interface isn't required to have methods that implement the interface.
InterfaceImplementation? iface = overrideInformation.InterfaceImplementor.InterfaceImplementation;
if (!((iface is not null && Annotations.IsMarked (iface))
|| IsInterfaceImplementationMarkedRecursively (method.DeclaringType, @base.DeclaringType)))
// We must check all possible ways the interface could be implemented by the type (through all recursive interface implementations, not just the primary one)
if (!(IsInterfaceImplementationMarkedRecursively (overrideInformation.RuntimeInterfaceImplementation.Implementor, @base.DeclaringType)))
return false;

// If the interface method is not marked and the interface doesn't come from a preserved scope, do not mark the implementation method
Expand All @@ -2529,12 +2535,12 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat
// If the method is static and the implementing type is relevant to variant casting, mark the implementation method.
// A static method may only be called through a constrained call if the type is relevant to variant casting.
if (@base.IsStatic)
return Annotations.IsRelevantToVariantCasting (overrideInformation.InterfaceImplementor.Implementor)
return Annotations.IsRelevantToVariantCasting (overrideInformation.RuntimeInterfaceImplementation.Implementor)
|| IgnoreScope (@base.DeclaringType.Scope);

// If the implementing type is marked as instantiated, mark the implementation method.
// If the type is not instantiated, do not mark the implementation method
return Annotations.IsInstantiated (overrideInformation.InterfaceImplementor.Implementor);
return Annotations.IsInstantiated (overrideInformation.RuntimeInterfaceImplementation.Implementor);
}

static bool IsSpecialSerializationConstructor (MethodDefinition method)
Expand Down Expand Up @@ -2597,31 +2603,18 @@ void MarkCustomMarshalerGetInstance (TypeDefinition type, in DependencyInfo reas

void MarkICustomMarshalerMethods (TypeDefinition inputType, in DependencyInfo reason, MessageOrigin origin)
{
TypeDefinition? type = inputType;
do {
if (!type.HasInterfaces)
continue;

foreach (var iface in type.Interfaces) {
var iface_type = iface.InterfaceType;
if (!iface_type.IsTypeOf ("System.Runtime.InteropServices", "ICustomMarshaler"))
continue;

//
// Instead of trying to guess where to find the interface declaration ILLink walks
// the list of implemented interfaces and resolve the declaration from there
//
var tdef = Context.Resolve (iface_type);
if (tdef == null) {
return;
}
var runtimeInterfaces = Annotations.GetRuntimeInterfaces (inputType);
if (runtimeInterfaces is null)
return;
foreach (var runtimeInterface in runtimeInterfaces) {

MarkMethodsIf (tdef.Methods, m => !m.IsStatic, reason, origin);
var iface_type = runtimeInterface.InterfaceTypeDefinition;
if (false == iface_type?.IsTypeOf ("System.Runtime.InteropServices", "ICustomMarshaler"))
continue;
MarkMethodsIf (iface_type!.Methods, m => !m.IsStatic, reason, origin);

MarkInterfaceImplementation (iface, new MessageOrigin (type));
return;
}
} while ((type = Context.TryResolve (type.BaseType)) != null);
MarkRuntimeInterfaceImplementation (runtimeInterface, new MessageOrigin (Context.Resolve (runtimeInterface.TypeWithInterfaceImplementation)));
}
}

bool IsNonEmptyStaticConstructor (MethodDefinition method)
Expand Down Expand Up @@ -3315,13 +3308,13 @@ void MarkRuntimeInterfaceImplementation (MethodDefinition method, MethodReferenc
if (!resolvedOverride.DeclaringType.IsInterface)
return;
var interfaceToBeImplemented = ov.DeclaringType;

var ifaces = Annotations.GetRecursiveInterfaces (method.DeclaringType);
var ifaces = Annotations.GetRuntimeInterfaces (method.DeclaringType);
if (ifaces is null)
return;
Debug.Assert (ifaces.Value.SingleOrDefault (i => TypeReferenceEqualityComparer.AreEqual (i.InflatedInterfaceType, interfaceToBeImplemented, Context)) is not null);
foreach (var iface in ifaces) {
if (TypeReferenceEqualityComparer.AreEqual (iface.InterfaceType, interfaceToBeImplemented, Context)) {
MarkInterfaceImplementationList (iface.ImplementationChain, new MessageOrigin (method.DeclaringType));
if (TypeReferenceEqualityComparer.AreEqual (iface.InflatedInterfaceType, interfaceToBeImplemented, Context)) {
MarkRuntimeInterfaceImplementation (iface, new MessageOrigin (method.DeclaringType));
return;
}
}
Expand Down Expand Up @@ -3645,7 +3638,7 @@ void MarkInterfacesNeededByBodyStack (MethodIL methodIL)
return;

foreach (var (implementation, type) in implementations)
MarkInterfaceImplementation (implementation, new MessageOrigin (type));
MarkRuntimeInterfaceImplementation (implementation, new MessageOrigin (type));
}

bool InstructionRequiresReflectionMethodBodyScannerForFieldAccess (Instruction instruction)
Expand Down Expand Up @@ -3753,9 +3746,9 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
}
}

void MarkInterfaceImplementationList (List<InterfaceImplementation> ifaces, MessageOrigin origin, DependencyInfo? reason = null)
void MarkRuntimeInterfaceImplementation (RuntimeInterfaceImplementation runtimeInterface, MessageOrigin origin, DependencyInfo? reason = null)
{
foreach (var iface in ifaces) {
foreach (var iface in runtimeInterface.InterfaceImplementation) {
MarkInterfaceImplementation (iface, origin, reason);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/illink/src/linker/Linker/Annotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection.Metadata.Ecma335;
using ILLink.Shared.TrimAnalysis;
using Mono.Cecil;
using Mono.Cecil.Cil;
Expand Down Expand Up @@ -719,7 +719,7 @@ public void EnqueueVirtualMethod (MethodDefinition method)
VirtualMethodsWithAnnotationsToValidate.Add (method);
}

internal List<(TypeReference InterfaceType, List<InterfaceImplementation> ImplementationChain)>? GetRecursiveInterfaces (TypeDefinition type)
internal ImmutableArray<RuntimeInterfaceImplementation>? GetRuntimeInterfaces (TypeDefinition type)
{
return TypeMapInfo.GetRecursiveInterfaces (type);
}
Expand Down
59 changes: 0 additions & 59 deletions src/tools/illink/src/linker/Linker/InterfaceImplementor.cs
View file
Open in desktop

This file was deleted.

31 changes: 8 additions & 23 deletions src/tools/illink/src/linker/Linker/MethodBodyScanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public InterfacesOnStackScanner (LinkContext context)
this.context = context;
}

public IEnumerable<(InterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodIL methodIL)
public IEnumerable<(RuntimeInterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodIL methodIL)
{
var possibleStackTypes = AllPossibleStackTypes (methodIL);
if (possibleStackTypes.Count == 0)
Expand All @@ -73,7 +73,7 @@ public InterfacesOnStackScanner (LinkContext context)
if (interfaceTypes.Length == 0)
return null;

var interfaceImplementations = new HashSet<(InterfaceImplementation, TypeDefinition)> ();
var interfaceImplementations = new HashSet<(RuntimeInterfaceImplementation, TypeDefinition)> ();

// If a type could be on the stack in the body and an interface it implements could be on the stack on the body
// then we need to mark that interface implementation. When this occurs it is not safe to remove the interface implementation from the type
Expand Down Expand Up @@ -139,31 +139,16 @@ HashSet<TypeDefinition> AllPossibleStackTypes (MethodIL methodIL)
return types;
}

void AddMatchingInterfaces (HashSet<(InterfaceImplementation, TypeDefinition)> results, TypeDefinition type, TypeDefinition[] interfaceTypes)
void AddMatchingInterfaces (HashSet<(RuntimeInterfaceImplementation, TypeDefinition)> results, TypeDefinition type, TypeDefinition[] interfaceTypes)
{
if (!type.HasInterfaces)
var runtimeInterfaces = context.Annotations.GetRuntimeInterfaces (type);
if (runtimeInterfaces is null)
return;

foreach (var interfaceType in interfaceTypes) {
if (HasInterface (type, interfaceType, out InterfaceImplementation? implementation))
results.Add ((implementation, type));
}
}

bool HasInterface (TypeDefinition type, TypeDefinition interfaceType, [NotNullWhen (true)] out InterfaceImplementation? implementation)
{
implementation = null;
if (!type.HasInterfaces)
return false;

foreach (var iface in type.Interfaces) {
if (context.TryResolve (iface.InterfaceType) == interfaceType) {
implementation = iface;
return true;
foreach (var iface in runtimeInterfaces) {
if (interfaceTypes.Contains (iface.InterfaceTypeDefinition)) {
results.Add ((iface, type));
}
}

return false;
}

void AddFromGenericInstance (HashSet<TypeDefinition> set, IGenericInstance instance)
Expand Down
Loading
Loading