Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
17c2fd7
Mark all base types and interfaces as RelevantToVariantCasting
jtschuster Jan 25, 2024
642385d
Update Test infra
jtschuster Jan 27, 2024
565837a
Mark static DIM if it provides an implementation for a type that is r…
jtschuster Jan 30, 2024
c3b0c63
Remove unused code and optimize default interface method marking
jtschuster Feb 1, 2024
e6d3788
Revert AssemblyChecker changes
jtschuster Feb 1, 2024
bb67ff1
Add example of overmarking in test
jtschuster Feb 1, 2024
8b8a83d
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster Feb 1, 2024
8851aae
wip
jtschuster Feb 3, 2024
e5b4a19
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster Feb 3, 2024
4beb771
Keep all DIMs that provide an implementation for a kept interface method
jtschuster Feb 5, 2024
a46a642
Add generated tests
jtschuster Feb 5, 2024
601658a
Fix test expectations
jtschuster Feb 5, 2024
5520d85
Use ProcessDefaultImplementations for static iface methods
jtschuster Feb 5, 2024
64e9f86
Undo unrelated changes
jtschuster Feb 5, 2024
57ca7d1
Get rid of _interfaceOverrides, use Annotations.GetOverrides and Anno…
jtschuster Feb 6, 2024
c47c7d3
Clean up changes
jtschuster Feb 6, 2024
694443a
Undo moving lines, update doc comments
jtschuster Feb 6, 2024
1f0cdee
Merge branch 'main' into StaticDimFix
jtschuster Feb 6, 2024
42d8764
Remove redundant test
jtschuster Feb 7, 2024
386f644
Add test types to make sure all DIMs aren't kept
jtschuster Feb 8, 2024
cab167f
PR feedback
jtschuster Feb 8, 2024
bc4470f
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster Feb 10, 2024
51be916
Break early if the DIM is the interface method
jtschuster Feb 10, 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
42 changes: 22 additions & 20 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ void ProcessVirtualMethod (MethodDefinition method)
var defaultImplementations = Annotations.GetDefaultInterfaceImplementations (method);
if (defaultImplementations != null) {
foreach (var defaultImplementationInfo in defaultImplementations) {
ProcessDefaultImplementation (defaultImplementationInfo.InstanceType, defaultImplementationInfo.ProvidingInterface);
ProcessDefaultImplementation (defaultImplementationInfo.ImplementingType, defaultImplementationInfo.InterfaceImpl, defaultImplementationInfo.DefaultInterfaceMethod);
}
}
}
Expand All @@ -724,11 +724,11 @@ bool ShouldMarkOverrideForBase (OverrideInformation overrideInformation)
Debug.Assert (Annotations.IsMarked (overrideInformation.Base) || IgnoreScope (overrideInformation.Base.DeclaringType.Scope));
if (!Annotations.IsMarked (overrideInformation.Override.DeclaringType))
return false;

if (overrideInformation.IsOverrideOfInterfaceMember) {
_interfaceOverrides.Add ((overrideInformation, ScopeStack.CurrentScope));
return false;
}

if (!Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, overrideInformation.Override))
return true;

Expand Down Expand Up @@ -816,11 +816,15 @@ bool RequiresInterfaceRecursively (TypeDefinition typeToExamine, TypeDefinition
return false;
}

void ProcessDefaultImplementation (TypeDefinition typeWithDefaultImplementedInterfaceMethod, InterfaceImplementation implementation)
void ProcessDefaultImplementation (TypeDefinition typeWithDefaultImplementedInterfaceMethod, InterfaceImplementation implementation, MethodDefinition implementationMethod)
{
if (!Annotations.IsInstantiated (typeWithDefaultImplementedInterfaceMethod))
if ((!implementationMethod.IsStatic && !Annotations.IsInstantiated (typeWithDefaultImplementedInterfaceMethod))
|| implementationMethod.IsStatic && !Annotations.IsRelevantToVariantCasting(typeWithDefaultImplementedInterfaceMethod))
return;

var origin = ScopeStack.CurrentScope.Origin;
MarkMethod(implementationMethod, new DependencyInfo(DependencyKind.Unspecified, implementation), in origin);

MarkInterfaceImplementation (implementation);
}

Expand Down Expand Up @@ -2275,9 +2279,9 @@ void MarkTypeWithDebuggerDisplayAttribute (TypeDefinition type, CustomAttribute
// Record a logical dependency on the attribute so that we can blame it for the kept members below.
Tracer.AddDirectDependency (attribute, new DependencyInfo (DependencyKind.CustomAttribute, type), marked: false);

MarkTypeWithDebuggerDisplayAttributeValue(type, attribute, (string) attribute.ConstructorArguments[0].Value);
MarkTypeWithDebuggerDisplayAttributeValue (type, attribute, (string) attribute.ConstructorArguments[0].Value);
if (attribute.HasProperties) {
foreach (var property in attribute.Properties) {
foreach (var property in attribute.Properties) {
if (property.Name is "Name" or "Type") {
MarkTypeWithDebuggerDisplayAttributeValue (type, attribute, (string) property.Argument.Value);
}
Expand Down Expand Up @@ -2549,15 +2553,13 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat
{
var @base = overrideInformation.Base;
var method = overrideInformation.Override;
Debug.Assert (@base.DeclaringType.IsInterface);
if (@base is null || method is null || @base.DeclaringType is null)
return false;

if (Annotations.IsMarked (method))
return false;

if ([email protected])
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.MatchingInterfaceImplementation;
Expand All @@ -2572,7 +2574,7 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat

// If the interface method is abstract, mark the implementation method
// The method is needed for valid IL.
if (@base.IsAbstract)
if (@base.IsAbstract && [email protected])
return true;

// If the method is static and the implementing type is relevant to variant casting, mark the implementation method.
Expand Down Expand Up @@ -3007,7 +3009,7 @@ void MarkMethodCollection (IList<MethodDefinition> methods, in DependencyInfo re
protected virtual MethodDefinition? MarkMethod (MethodReference reference, DependencyInfo reason, in MessageOrigin origin)
{
DependencyKind originalReasonKind = reason.Kind;
(reference, reason) = GetOriginalMethod (reference, reason);
(reference, reason) = GetOriginalMethod (reference, reason, in origin);

if (reference.DeclaringType is ArrayType arrayType) {
MarkType (reference.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, reference));
Expand Down Expand Up @@ -3178,14 +3180,15 @@ internal static void ReportRequiresUnreferencedCode (string displayName, Require
diagnosticContext.AddDiagnostic (DiagnosticId.RequiresUnreferencedCode, displayName, arg1, arg2);
}

protected (MethodReference, DependencyInfo) GetOriginalMethod (MethodReference method, DependencyInfo reason)
protected (MethodReference, DependencyInfo) GetOriginalMethod (MethodReference method, DependencyInfo reason, in MessageOrigin origin)
{
while (method is MethodSpecification specification) {
// Blame the method reference (which isn't marked) on the original reason.
Tracer.AddDirectDependency (specification, reason, marked: false);
// Blame the outgoing element method on the specification.
if (method is GenericInstanceMethod gim)
if (method is GenericInstanceMethod gim) {
MarkGenericArguments (gim);
}

(method, reason) = (specification.ElementMethod, new DependencyInfo (DependencyKind.ElementMethod, specification));
Debug.Assert (!(method is MethodSpecification));
Expand Down Expand Up @@ -3231,7 +3234,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
} else if (method.TryGetProperty (out PropertyDefinition? property))
MarkProperty (property, new DependencyInfo (PropagateDependencyKindToAccessors (reason.Kind, DependencyKind.PropertyOfPropertyMethod), method));
else if (method.TryGetEvent (out EventDefinition? @event)) {
MarkEvent (@event, new DependencyInfo (PropagateDependencyKindToAccessors(reason.Kind, DependencyKind.EventOfEventMethod), method));
MarkEvent (@event, new DependencyInfo (PropagateDependencyKindToAccessors (reason.Kind, DependencyKind.EventOfEventMethod), method));
}

if (method.HasMetadataParameters ()) {
Expand Down Expand Up @@ -3315,7 +3318,7 @@ protected virtual void DoAdditionalMethodProcessing (MethodDefinition method)
{
}

static DependencyKind PropagateDependencyKindToAccessors(DependencyKind parentDependencyKind, DependencyKind kind)
static DependencyKind PropagateDependencyKindToAccessors (DependencyKind parentDependencyKind, DependencyKind kind)
{
switch (parentDependencyKind) {
// If the member is marked due to descriptor or similar, propagate the original reason to suppress some warnings correctly
Expand All @@ -3335,11 +3338,11 @@ void MarkImplicitlyUsedFields (TypeDefinition type)
return;

// keep fields for types with explicit layout, for enums and for InlineArray types
if (!type.IsAutoLayout || type.IsEnum || TypeIsInlineArrayType(type))
if (!type.IsAutoLayout || type.IsEnum || TypeIsInlineArrayType (type))
MarkFields (type, includeStatic: type.IsEnum, reason: new DependencyInfo (DependencyKind.MemberOfType, type));
}

static bool TypeIsInlineArrayType(TypeDefinition type)
static bool TypeIsInlineArrayType (TypeDefinition type)
{
if (!type.IsValueType)
return false;
Expand Down Expand Up @@ -3584,7 +3587,7 @@ protected internal virtual void MarkEvent (EventDefinition evt, in DependencyInf

MarkCustomAttributes (evt, new DependencyInfo (DependencyKind.CustomAttribute, evt));

DependencyKind dependencyKind = PropagateDependencyKindToAccessors(reason.Kind, DependencyKind.EventMethod);
DependencyKind dependencyKind = PropagateDependencyKindToAccessors (reason.Kind, DependencyKind.EventMethod);
MarkMethodIfNotNull (evt.AddMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin);
MarkMethodIfNotNull (evt.InvokeMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin);
MarkMethodIfNotNull (evt.RemoveMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin);
Expand Down Expand Up @@ -3762,8 +3765,7 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
ScopeStack.UpdateCurrentScopeInstructionOffset (instruction.Offset);
if (markForReflectionAccess) {
MarkMethodVisibleToReflection (methodReference, new DependencyInfo (dependencyKind, method), ScopeStack.CurrentScope.Origin);
}
else {
} else {
MarkMethod (methodReference, new DependencyInfo (dependencyKind, method), ScopeStack.CurrentScope.Origin);
}
break;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/illink/src/linker/Linker/Annotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,9 @@ public bool IsPublic (IMetadataTokenProvider provider)
return TypeMapInfo.GetOverrides (method);
}

public IEnumerable<(TypeDefinition InstanceType, InterfaceImplementation ProvidingInterface)>? GetDefaultInterfaceImplementations (MethodDefinition method)
public IEnumerable<(TypeDefinition ImplementingType, InterfaceImplementation InterfaceImpl, MethodDefinition DefaultInterfaceMethod)> GetDefaultInterfaceImplementations (MethodDefinition method)
{
return TypeMapInfo.GetDefaultInterfaceImplementations (method);
return TypeMapInfo.GetDefaultInterfaceImplementations (method) ?? [];
}

/// <summary>
Expand Down
18 changes: 9 additions & 9 deletions src/tools/illink/src/linker/Linker/TypeMapInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class TypeMapInfo
readonly LinkContext context;
protected readonly Dictionary<MethodDefinition, List<OverrideInformation>> base_methods = new Dictionary<MethodDefinition, List<OverrideInformation>> ();
protected readonly Dictionary<MethodDefinition, List<OverrideInformation>> override_methods = new Dictionary<MethodDefinition, List<OverrideInformation>> ();
protected readonly Dictionary<MethodDefinition, List<(TypeDefinition InstanceType, InterfaceImplementation ImplementationProvider)>> default_interface_implementations = new Dictionary<MethodDefinition, List<(TypeDefinition, InterfaceImplementation)>> ();
protected readonly Dictionary<MethodDefinition, List<(TypeDefinition InstanceType, InterfaceImplementation ImplementationProvider, MethodDefinition DefaultImplementationMethod)>> default_interface_implementations = new Dictionary<MethodDefinition, List<(TypeDefinition, InterfaceImplementation, MethodDefinition)>> ();

public TypeMapInfo (LinkContext context)
{
Expand Down Expand Up @@ -84,9 +84,9 @@ public void EnsureProcessed (AssemblyDefinition assembly)
return bases;
}

public IEnumerable<(TypeDefinition InstanceType, InterfaceImplementation ProvidingInterface)>? GetDefaultInterfaceImplementations (MethodDefinition method)
public IEnumerable<(TypeDefinition ImplementingType, InterfaceImplementation InterfaceImpl, MethodDefinition DefaultImplementationMethod)>? GetDefaultInterfaceImplementations (MethodDefinition baseMethod)
{
default_interface_implementations.TryGetValue (method, out var ret);
default_interface_implementations.TryGetValue (baseMethod, out var ret);
return ret;
}

Expand All @@ -110,14 +110,14 @@ public void AddOverride (MethodDefinition @base, MethodDefinition @override, Int
methods.Add (new OverrideInformation (@base, @override, context, matchingInterfaceImplementation));
}

public void AddDefaultInterfaceImplementation (MethodDefinition @base, TypeDefinition implementingType, InterfaceImplementation matchingInterfaceImplementation)
public void AddDefaultInterfaceImplementation (MethodDefinition @base, TypeDefinition implementingType, (InterfaceImplementation, MethodDefinition) matchingInterfaceImplementation)
{
if (!default_interface_implementations.TryGetValue (@base, out var implementations)) {
implementations = new List<(TypeDefinition, InterfaceImplementation)> ();
implementations = new List<(TypeDefinition, InterfaceImplementation, MethodDefinition)> ();
default_interface_implementations.Add (@base, implementations);
}

implementations.Add ((implementingType, matchingInterfaceImplementation));
implementations.Add ((implementingType, matchingInterfaceImplementation.Item1, matchingInterfaceImplementation.Item2));
}

protected virtual void MapType (TypeDefinition type)
Expand Down Expand Up @@ -276,7 +276,7 @@ void AnnotateMethods (MethodDefinition @base, MethodDefinition @override, Interf
// Note that this returns a list to potentially cover the diamond case (more than one
// most specific implementation of the given interface methods). ILLink needs to preserve
// all the implementations so that the proper exception can be thrown at runtime.
IEnumerable<InterfaceImplementation> GetDefaultInterfaceImplementations (TypeDefinition type, MethodDefinition interfaceMethod)
IEnumerable<(InterfaceImplementation, MethodDefinition)> GetDefaultInterfaceImplementations (TypeDefinition type, MethodDefinition interfaceMethod)
{
// Go over all interfaces, trying to find a method that is an explicit MethodImpl of the
// interface method in question.
Expand All @@ -290,7 +290,7 @@ IEnumerable<InterfaceImplementation> GetDefaultInterfaceImplementations (TypeDef
foreach (var potentialImplMethod in potentialImplInterface.Methods) {
if (potentialImplMethod == interfaceMethod &&
!potentialImplMethod.IsAbstract) {
yield return interfaceImpl;
yield return (interfaceImpl, potentialImplMethod);
}

if (!potentialImplMethod.HasOverrides)
Expand All @@ -299,7 +299,7 @@ IEnumerable<InterfaceImplementation> GetDefaultInterfaceImplementations (TypeDef
// This method is an override of something. Let's see if it's the method we are looking for.
foreach (var @override in potentialImplMethod.Overrides) {
if (context.TryResolve (@override) == interfaceMethod) {
yield return interfaceImpl;
yield return (interfaceImpl, potentialImplMethod);
foundImpl = true;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,36 @@ public Task InterfaceWithAttributeOnImplementation ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MostSpecificDefaultImplementationKeptInstance ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MostSpecificDefaultImplementationKeptStatic ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task SimpleDefaultInterfaceMethod ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task StaticDefaultInterfaceMethodOnDerivedInterface ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task StaticDefaultInterfaceMethodOnStruct ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task UnusedDefaultInterfaceImplementation ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ public Task LibraryWithUnresolvedInterfaces ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task RootAllLibraryBehavior ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task RootAllLibraryCopyBehavior ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task RootLibrary ()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
using Mono.Linker.Tests.Cases.Expectations.Assertions;

namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.DefaultInterfaceMethods
{
[TestCaseRequirements (TestRunCharacteristics.SupportsDefaultInterfaceMethods, "Requires support for default interface methods")]
class MostSpecificDefaultImplementationKeptInstance
{
[Kept]
public static void Main ()
{
M (new UsedAsIBase());
}


[Kept]
static int M (IBase ibase)
{
return ibase.Value;
}

[Kept]
interface IBase
{
[Kept]
int Value {
[Kept]
get => 0;
}

int Value2 {
get => 0;
}
}

[Kept]
[KeptInterface (typeof (IBase))]
interface IMiddle : IBase
{
[Kept]
int IBase.Value {
[Kept]
get => 1;
}

int Value2 {
get => 0;
}
}

[Kept]
[KeptInterface (typeof (IBase))]
[KeptInterface (typeof (IMiddle))]
interface IDerived : IMiddle
{
[Kept]
int IBase.Value {
[Kept]
get => 2;
}

int Value2 {
get => 0;
}
}

interface INotReferenced
{ }

[Kept]
[KeptInterface (typeof (IDerived))]
[KeptInterface (typeof (IMiddle))]
[KeptInterface (typeof (IBase))]
[KeptMember(".ctor()")]
class UsedAsIBase : IDerived, INotReferenced
{
}
}
}
Loading