Skip to content
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.

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

[ILLink] Add interface implementing type to OverrideInformation and use where it makes sense #98274

Merged
merged 20 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
8454427
Refactor OverrideInformation to include interface implementing type
jtschuster Feb 11, 2024
54c326d
Add NotNullWhenAttribute for InterfaceImplementor
jtschuster Feb 11, 2024
6bfc58a
Don't allow null interfaceImpl when base and override are interface m…
jtschuster Feb 11, 2024
ecc5d0c
Add InterfaceType to InterfaceImplementor
jtschuster Feb 11, 2024
310de30
Make new members internal for compat warnings
jtschuster Feb 13, 2024
77195a3
Make InterfaceImplementor internal for API compat
jtschuster Feb 14, 2024
a73cc1f
Make TypeMapInfo internal for API Compat
jtschuster Feb 14, 2024
76bc255
Add InterfaceImplementor to suppressions.xml
jtschuster Feb 14, 2024
11577fa
Make types public again
jtschuster Feb 14, 2024
5fb3256
Use correct InterfaceImplementor in FindAndAddDims
jtschuster Feb 15, 2024
3b4a69d
InterfaceImplementor doesn't need to directly implement the interface
jtschuster Feb 15, 2024
14814ed
PR Feedback
jtschuster Feb 21, 2024
56f23e4
Don't need to resolve TypeDefinitions
jtschuster Feb 21, 2024
ccd0a8f
Merge branch 'main' of https://github.com/dotnet/runtime into CheckIm…
jtschuster Feb 22, 2024
1d1e883
Use correct InterfaceImpl
jtschuster Feb 22, 2024
7b218b6
Add argument to first call
jtschuster Feb 22, 2024
9addd27
Merge branch 'main' of https://github.com/dotnet/runtime into CheckIm…
jtschuster Feb 23, 2024
81ebc93
Look on base types and interfaces for interfaceImpl
jtschuster Feb 23, 2024
2110159
Add test case and invert if
jtschuster Feb 28, 2024
488ff72
PR feedback:
jtschuster Feb 29, 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
1 change: 1 addition & 0 deletions src/tools/illink/illink.sln
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ Global
SolutionGuid = {E43A3901-42B0-48CA-BB36-5CD40A99A6EE}
EndGlobalSection
GlobalSection(SharedMSBuildProjectFiles) = preSolution
test\Trimming.Tests.Shared\Trimming.Tests.Shared.projitems*{400a1561-b6b6-482d-9e4c-3ddaede5bd07}*SharedItemsImports = 5
src\ILLink.Shared\ILLink.Shared.projitems*{dd28e2b1-057b-4b4d-a04d-b2ebd9e76e46}*SharedItemsImports = 5
src\ILLink.Shared\ILLink.Shared.projitems*{f1a44a78-34ee-408b-8285-9a26f0e7d4f2}*SharedItemsImports = 5
src\ILLink.Shared\ILLink.Shared.projitems*{ff598e93-8e9e-4091-9f50-61a7572663ae}*SharedItemsImports = 13
Expand Down
8 changes: 4 additions & 4 deletions src/tools/illink/src/linker/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Mono.Linker.ILogger</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Mono.Linker.InterfaceImplementor</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:Mono.Linker.InternalErrorException</Target>
Expand Down Expand Up @@ -1481,10 +1485,6 @@
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.OverrideInformation.get_IsOverrideOfInterfaceMember</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.OverrideInformation.get_IsStaticInterfaceMethodPair</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.Steps.BaseStep.get_MarkingHelpers</Target>
Expand Down
32 changes: 16 additions & 16 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -701,17 +701,16 @@ void ProcessVirtualMethod (MethodDefinition method)
var defaultImplementations = Annotations.GetDefaultInterfaceImplementations (method);
if (defaultImplementations is not null) {
foreach (var dimInfo in defaultImplementations) {
ProcessDefaultImplementation (dimInfo.ImplementingType, dimInfo.InterfaceImpl, dimInfo.DefaultInterfaceMethod);
ProcessDefaultImplementation (dimInfo);

var ov = new OverrideInformation (method, dimInfo.DefaultInterfaceMethod, Context, dimInfo.InterfaceImpl);
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, dimInfo.ImplementingType))
MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (dimInfo))
MarkMethod (dimInfo.Override, new DependencyInfo (DependencyKind.Override, dimInfo.Base), ScopeStack.CurrentScope.Origin);
}
}
var overridingMethods = Annotations.GetOverrides (method);
if (overridingMethods is not null) {
foreach (var ov in overridingMethods) {
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, ov.Override.DeclaringType))
foreach (OverrideInformation ov in overridingMethods) {
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov))
MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
}
}
Expand Down Expand Up @@ -819,13 +818,14 @@ bool RequiresInterfaceRecursively (TypeDefinition typeToExamine, TypeDefinition
return false;
}

void ProcessDefaultImplementation (TypeDefinition typeWithDefaultImplementedInterfaceMethod, InterfaceImplementation implementation, MethodDefinition implementationMethod)
void ProcessDefaultImplementation (OverrideInformation ov)
{
if ((!implementationMethod.IsStatic && !Annotations.IsInstantiated (typeWithDefaultImplementedInterfaceMethod))
|| implementationMethod.IsStatic && !Annotations.IsRelevantToVariantCasting (typeWithDefaultImplementedInterfaceMethod))
Debug.Assert (ov.IsOverrideOfInterfaceMember);
if ((!ov.Override.IsStatic && !Annotations.IsInstantiated (ov.InterfaceImplementor.Implementor))
|| ov.Override.IsStatic && !Annotations.IsRelevantToVariantCasting (ov.InterfaceImplementor.Implementor))
return;

MarkInterfaceImplementation (implementation);
MarkInterfaceImplementation (ov.InterfaceImplementor.InterfaceImplementation);
}

void MarkMarshalSpec (IMarshalInfoProvider spec, in DependencyInfo reason)
Expand Down Expand Up @@ -2549,11 +2549,11 @@ bool IsMethodNeededByTypeDueToPreservedScope (MethodDefinition method)
/// <summary>
/// Returns true if the override method is required due to the interface that the base method is declared on. See doc at <see href="docs/methods-kept-by-interface.md"/> for explanation of logic.
/// </summary>
bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformation overrideInformation, TypeDefinition typeThatImplsInterface)
bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformation overrideInformation)
{
var @base = overrideInformation.Base;
var method = overrideInformation.Override;
Debug.Assert (@base.DeclaringType.IsInterface);
Debug.Assert (overrideInformation.IsOverrideOfInterfaceMember);
if (@base is null || method is null || @base.DeclaringType is null)
return false;

Expand All @@ -2562,7 +2562,7 @@ bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformat

// 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;
InterfaceImplementation? iface = overrideInformation.InterfaceImplementor.InterfaceImplementation;
if (!((iface is not null && Annotations.IsMarked (iface))
|| IsInterfaceImplementationMarkedRecursively (method.DeclaringType, @base.DeclaringType)))
return false;
Expand All @@ -2580,12 +2580,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 (typeThatImplsInterface)
return Annotations.IsRelevantToVariantCasting (overrideInformation.InterfaceImplementor.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 (typeThatImplsInterface);
return Annotations.IsInstantiated (overrideInformation.InterfaceImplementor.Implementor);
}

static bool IsSpecialSerializationConstructor (MethodDefinition method)
Expand Down Expand Up @@ -3256,7 +3256,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
// Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round.
if (!markAllOverrides &&
Context.Resolve (@base) is MethodDefinition baseDefinition
&& new OverrideInformation.OverridePair (baseDefinition, method).IsStaticInterfaceMethodPair ())
&& baseDefinition.DeclaringType.IsInterface && baseDefinition.IsStatic && method.IsStatic)
continue;
MarkMethod (@base, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin);
MarkExplicitInterfaceImplementation (method, @base);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/illink/src/linker/Linker/Annotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public bool IsPublic (IMetadataTokenProvider provider)
/// DefaultInterfaceMethod is the method that implements <paramref name="method"/>.
/// </summary>
/// <param name="method">The interface method to find default implementations for</param>
public IEnumerable<(TypeDefinition ImplementingType, InterfaceImplementation InterfaceImpl, MethodDefinition DefaultInterfaceMethod)>? GetDefaultInterfaceImplementations (MethodDefinition method)
public IEnumerable<OverrideInformation>? GetDefaultInterfaceImplementations (MethodDefinition method)
{
return TypeMapInfo.GetDefaultInterfaceImplementations (method);
}
Expand Down
68 changes: 68 additions & 0 deletions src/tools/illink/src/linker/Linker/InterfaceImplementor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using Mono.Cecil;

namespace Mono.Linker
{
public class InterfaceImplementor
{
/// <summary>
/// The type that implements <see cref="InterfaceImplementor.InterfaceType"/>.
/// </summary>
public TypeDefinition Implementor { get; }
jtschuster marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// The .interfaceimpl on <see cref="InterfaceImplementor.Implementor"/>that points to <see cref="InterfaceImplementor.InterfaceType"/>
/// </summary>
public InterfaceImplementation InterfaceImplementation { get; }
/// <summary>
/// The type of the interface that is implemented by <see cref="InterfaceImplementor.Implementor"/>
/// </summary>
public TypeDefinition InterfaceType { get; }
jtschuster marked this conversation as resolved.
Show resolved Hide resolved

public InterfaceImplementor (TypeDefinition implementor, InterfaceImplementation interfaceImplementation, TypeDefinition interfaceType)
{
Implementor = implementor;
InterfaceImplementation = interfaceImplementation;
InterfaceType = interfaceType;
}

public static InterfaceImplementor Create(TypeDefinition implementor, TypeDefinition interfaceType, IMetadataResolver resolver)
{
foreach(InterfaceImplementation iface in implementor.Interfaces) {
if (resolver.Resolve(iface.InterfaceType) == interfaceType) {
return new InterfaceImplementor(implementor, iface, interfaceType);
}
}
var baseTypeRef = implementor.BaseType;
while (baseTypeRef is not null) {
sbomer marked this conversation as resolved.
Show resolved Hide resolved
var baseType = resolver.Resolve (baseTypeRef);
foreach(InterfaceImplementation iface in baseType.Interfaces) {
if (resolver.Resolve(iface.InterfaceType) == interfaceType) {
return new InterfaceImplementor(implementor, iface, interfaceType);
}
}
baseTypeRef = baseType.BaseType;
}

Queue<TypeDefinition> ifacesToCheck = new ();
ifacesToCheck.Enqueue(implementor);
while (ifacesToCheck.Count > 0) {
var myFace = ifacesToCheck.Dequeue ();
jtschuster marked this conversation as resolved.
Show resolved Hide resolved

foreach(InterfaceImplementation ifaceImpl in myFace.Interfaces) {
var iface = resolver.Resolve (ifaceImpl.InterfaceType);
if (iface == interfaceType) {
return new InterfaceImplementor(implementor, ifaceImpl, interfaceType);
}
ifacesToCheck.Enqueue (iface);
}
}
throw new InvalidOperationException ($"Type '{implementor.FullName}' does not implement interface '{interfaceType.FullName}' directly or through any base types or interfaces");
}
}
}
74 changes: 21 additions & 53 deletions src/tools/illink/src/linker/Linker/OverrideInformation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,71 +3,39 @@

using System.Diagnostics;
using Mono.Cecil;
using System.Diagnostics.CodeAnalysis;

namespace Mono.Linker
{
[DebuggerDisplay ("{Override}")]
public class OverrideInformation
{
readonly ITryResolveMetadata resolver;
readonly OverridePair _pair;
private InterfaceImplementation? _matchingInterfaceImplementation;
public MethodDefinition Base { get; }

public OverrideInformation (MethodDefinition @base, MethodDefinition @override, ITryResolveMetadata resolver, InterfaceImplementation? matchingInterfaceImplementation = null)
{
_pair = new OverridePair (@base, @override);
_matchingInterfaceImplementation = matchingInterfaceImplementation;
this.resolver = resolver;
}
public readonly record struct OverridePair (MethodDefinition Base, MethodDefinition Override)
{
public bool IsStaticInterfaceMethodPair () => Base.DeclaringType.IsInterface && Base.IsStatic && Override.IsStatic;
public InterfaceImplementation? GetMatchingInterfaceImplementation (ITryResolveMetadata resolver)
{
if (!Base.DeclaringType.IsInterface)
return null;
var interfaceType = Base.DeclaringType;
foreach (var @interface in Override.DeclaringType.Interfaces) {
if (resolver.TryResolve (@interface.InterfaceType)?.Equals (interfaceType) == true) {
return @interface;
}
}
return null;
}
}
public MethodDefinition Override { get; }

public MethodDefinition Base { get => _pair.Base; }
public MethodDefinition Override { get => _pair.Override; }
public InterfaceImplementation? MatchingInterfaceImplementation {
get {
if (_matchingInterfaceImplementation is not null)
return _matchingInterfaceImplementation;
_matchingInterfaceImplementation = _pair.GetMatchingInterfaceImplementation (resolver);
return _matchingInterfaceImplementation;
}
}
internal InterfaceImplementor? InterfaceImplementor { get; }

public bool IsOverrideOfInterfaceMember {
get {
if (MatchingInterfaceImplementation != null)
return true;

return Base.DeclaringType.IsInterface;
}
internal OverrideInformation (MethodDefinition @base, MethodDefinition @override, InterfaceImplementor? interfaceImplementor = null)
{
Base = @base;
Override = @override;
InterfaceImplementor = interfaceImplementor;
// Ensure we have an interface implementation if the base method is from an interface and the override method is on a class
Debug.Assert(@base.DeclaringType.IsInterface && interfaceImplementor != null
jtschuster marked this conversation as resolved.
Show resolved Hide resolved
|| [email protected] && interfaceImplementor == null);
// Ensure the interfaceImplementor is for the interface we expect
Debug.Assert (@base.DeclaringType.IsInterface ? interfaceImplementor!.InterfaceType == @base.DeclaringType : true);
}

public TypeDefinition? InterfaceType {
get {
if (!IsOverrideOfInterfaceMember)
return null;
public InterfaceImplementation? MatchingInterfaceImplementation
=> InterfaceImplementor?.InterfaceImplementation;

if (MatchingInterfaceImplementation != null)
return resolver.TryResolve (MatchingInterfaceImplementation.InterfaceType);

return Base.DeclaringType;
}
}
public TypeDefinition? InterfaceType
=> InterfaceImplementor?.InterfaceType;

public bool IsStaticInterfaceMethodPair => _pair.IsStaticInterfaceMethodPair ();
[MemberNotNullWhen (true, nameof (InterfaceImplementor), nameof (MatchingInterfaceImplementation))]
public bool IsOverrideOfInterfaceMember
=> InterfaceImplementor != null;
}
}
Loading
Loading