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.

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

Mark most specific static DIM for types marked RelevantToVariantCasting #97487

Merged
merged 23 commits into from
Feb 10, 2024
Merged

Mark most specific static DIM for types marked RelevantToVariantCasting #97487

Show file tree
Hide file tree
Changes from all 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
82 changes: 42 additions & 40 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 @@ -65,7 +65,6 @@ protected LinkContext Context {
readonly List<AttributeProviderPair> _ivt_attributes;
protected Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> _lateMarkedAttributes;
protected List<(TypeDefinition, MarkScopeStack.Scope)> _typesWithInterfaces;
protected HashSet<(OverrideInformation, MarkScopeStack.Scope)> _interfaceOverrides;
protected HashSet<AssemblyDefinition> _dynamicInterfaceCastableImplementationTypesDiscovered;
protected List<TypeDefinition> _dynamicInterfaceCastableImplementationTypes;
protected List<(MethodBody, MarkScopeStack.Scope)> _unreachableBodies;
Expand Down Expand Up @@ -226,7 +225,6 @@ public MarkStep ()
_ivt_attributes = new List<AttributeProviderPair> ();
_lateMarkedAttributes = new Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> ();
_typesWithInterfaces = new List<(TypeDefinition, MarkScopeStack.Scope)> ();
_interfaceOverrides = new HashSet<(OverrideInformation, MarkScopeStack.Scope)> ();
_dynamicInterfaceCastableImplementationTypesDiscovered = new HashSet<AssemblyDefinition> ();
_dynamicInterfaceCastableImplementationTypes = new List<TypeDefinition> ();
_unreachableBodies = new List<(MethodBody, MarkScopeStack.Scope)> ();
Expand Down Expand Up @@ -573,9 +571,10 @@ protected virtual void EnqueueMethod (MethodDefinition method, in DependencyInfo

void ProcessVirtualMethods ()
{
foreach ((MethodDefinition method, MarkScopeStack.Scope scope) in _virtual_methods) {
using (ScopeStack.PushScope (scope))
foreach ((var method, var scope) in _virtual_methods) {
using (ScopeStack.PushScope (scope)) {
ProcessVirtualMethod (method);
}
}
}

Expand Down Expand Up @@ -603,26 +602,19 @@ void ProcessMarkedTypesWithInterfaces ()
!unusedInterfacesOptimizationEnabled) {
MarkInterfaceImplementations (type);
}
// OverrideInformation for interfaces in PreservedScope aren't added yet
// Interfaces in PreservedScope should have their methods added to _virtual_methods so that they are properly processed
foreach (var method in type.Methods) {
var baseOverrideInformations = Annotations.GetBaseMethods (method);
if (baseOverrideInformations is null)
var baseMethods = Annotations.GetBaseMethods (method);
if (baseMethods is null)
continue;
foreach (var ov in baseOverrideInformations) {
if (ov.Base.DeclaringType is not null && ov.Base.DeclaringType.IsInterface && IgnoreScope (ov.Base.DeclaringType.Scope))
_interfaceOverrides.Add ((ov, ScopeStack.CurrentScope));
foreach (var ov in baseMethods) {
if (ov.Base.DeclaringType is not null && ov.Base.DeclaringType.IsInterface && IgnoreScope (ov.Base.DeclaringType.Scope)) {
_virtual_methods.Add ((ov.Base, ScopeStack.CurrentScope));
}
}
}
}
}

var interfaceOverrides = _interfaceOverrides.ToArray ();
foreach ((var overrideInformation, var scope) in interfaceOverrides) {
using (ScopeStack.PushScope (scope)) {
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (overrideInformation))
MarkMethod (overrideInformation.Override, new DependencyInfo (DependencyKind.Override, overrideInformation.Base), scope.Origin);
}
}
}

void DiscoverDynamicCastableImplementationInterfaces ()
Expand Down Expand Up @@ -705,10 +697,23 @@ void ProcessVirtualMethod (MethodDefinition method)
{
Annotations.EnqueueVirtualMethod (method);

var defaultImplementations = Annotations.GetDefaultInterfaceImplementations (method);
if (defaultImplementations != null) {
foreach (var defaultImplementationInfo in defaultImplementations) {
ProcessDefaultImplementation (defaultImplementationInfo.InstanceType, defaultImplementationInfo.ProvidingInterface);
if (method.DeclaringType.IsInterface) {
var defaultImplementations = Annotations.GetDefaultInterfaceImplementations (method);
if (defaultImplementations is not null) {
foreach (var dimInfo in defaultImplementations) {
ProcessDefaultImplementation (dimInfo.ImplementingType, dimInfo.InterfaceImpl, dimInfo.DefaultInterfaceMethod);

var ov = new OverrideInformation (method, dimInfo.DefaultInterfaceMethod, Context);
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, dimInfo.ImplementingType))
MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
}
}
var overridingMethods = Annotations.GetOverrides (method);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this might be causing the perf regression dotnet/sdk#40060, since it looks like we're newly walking the overrides of a virtual method every time we get here.

if (overridingMethods is not null) {
foreach (var ov in overridingMethods) {
if (IsInterfaceImplementationMethodNeededByTypeDueToInterface (ov, ov.Override.DeclaringType))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure using ov.Override.DeclaringType as the type with interfaceimpl is correct. I'm worried this could miss a case where we added an interface method due to IgnoreScope, and the type with interfaceimpl was passed as a generic arg, but the override declaring type is an interface that provides the default impl (but wasn't passed as a generic arg).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that situation be caught above when we loop over the default implementations of each interface method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that's true. It still worries me that the override declaring type isn't the same as the type with interfaceimpl that IsInterfaceImplementationMethodNeededByTypeDueToInterface expects. But I can't think of any problems with it. Maybe add a comment here in case we need to look at this again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this a little more, and it's another one of those "It works all the time, but not because the logic is correct". The DIM logic above handles static interface methods since they cannot be provided by a base type, only a DIM, but If a base method provides an instance method, we'll pass the wrong type in, but if the type implementing the interface is instantiated, the type providing the method for the interface will also be marked as instantiated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this fix is going to be more involved. I'll make an issue and make a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DIM logic above handles static interface methods since they cannot be provided by a base type

I think static interface methods can be provided by a base type: https://sharplab.io/#v2:EYLgtghglgdgNAFxAJwK7wCYgNQB8ACATAIwCwAUBQBICmANgA43IB0AIgPYA8bAfABQBKANwUKRAAS1GzCQG8KEpRPwBmFcQBsKgCwTOXACoDBEgO4ALZjQmGJICQEl5i5W8MsAskNHk3AXwpAynJYBGYAMwgAYxtnBT9lfC0JCGAAZwRkGIRdCW8RILFySQBhF0SlNQ1tfD0Circ3ZIBOfgAiWgBPdsLKiWDg8UJ9ewlSuCd5YKA==

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh, I really thought static interface methods couldn't be provided by the base, and I think that assumption might be made in the linker. I'll set up some test cases and see what's broken.

Copy link
Member Author

@jtschuster jtschuster Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the compiler injects a static interface method on D that call's C's method, so I think the assumption is valid. If the implementation on D is removed, ilasm can compile the IL, but it fails at runtime with a TypeLoadException:

Unhandled exception. System.TypeLoadException: Virtual static method 'M' is not implemented on type 'D' from assembly '_, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good to know. Thanks for checking!

MarkMethod (ov.Override, new DependencyInfo (DependencyKind.Override, ov.Base), ScopeStack.CurrentScope.Origin);
}
}
}
}
Expand All @@ -724,10 +729,8 @@ bool ShouldMarkOverrideForBase (OverrideInformation overrideInformation)
Debug.Assert (Annotations.IsMarked (overrideInformation.Base) || IgnoreScope (overrideInformation.Base.DeclaringType.Scope));
if (!Annotations.IsMarked (overrideInformation.Override.DeclaringType))
sbomer marked this conversation as resolved.
Show resolved Hide resolved
return false;
if (overrideInformation.IsOverrideOfInterfaceMember) {
_interfaceOverrides.Add ((overrideInformation, ScopeStack.CurrentScope));
if (overrideInformation.IsOverrideOfInterfaceMember)
return false;
}

if (!Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, overrideInformation.Override))
return true;
Expand Down Expand Up @@ -816,9 +819,10 @@ 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;

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 @@ -2545,19 +2549,17 @@ 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)
bool IsInterfaceImplementationMethodNeededByTypeDueToInterface (OverrideInformation overrideInformation, TypeDefinition typeThatImplsInterface)
{
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 @@ -2578,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 (method.DeclaringType)
return Annotations.IsRelevantToVariantCasting (typeThatImplsInterface)
|| 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 (method.DeclaringType);
return Annotations.IsInstantiated (typeThatImplsInterface);
}

static bool IsSpecialSerializationConstructor (MethodDefinition method)
Expand Down Expand Up @@ -3231,7 +3233,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 +3317,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 +3337,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 +3586,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
14 changes: 11 additions & 3 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 @@ -447,22 +447,30 @@ public bool IsPublic (IMetadataTokenProvider provider)
}

/// <summary>
/// Returns a list of all known methods that override <paramref name="method"/>. The list may be incomplete if other overrides exist in assemblies that haven't been processed by TypeMapInfo yet
/// Returns a list of all known methods that override <paramref name="method"/>.
/// The list may be incomplete if other overrides exist in assemblies that haven't been processed by TypeMapInfo yet
/// </summary>
public IEnumerable<OverrideInformation>? GetOverrides (MethodDefinition method)
{
return TypeMapInfo.GetOverrides (method);
}

public IEnumerable<(TypeDefinition InstanceType, InterfaceImplementation ProvidingInterface)>? GetDefaultInterfaceImplementations (MethodDefinition method)
/// <summary>
/// Returns a list of all default interface methods that implement <paramref name="method"/> for a type.
/// ImplementingType is the type that implements the interface,
/// InterfaceImpl is the <see cref="InterfaceImplementation" /> for the interface <paramref name="method" /> is declared on, and
/// 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)
{
return TypeMapInfo.GetDefaultInterfaceImplementations (method);
}

/// <summary>
/// Returns all base methods that <paramref name="method"/> overrides.
/// This includes methods on <paramref name="method"/>'s declaring type's base type (but not methods higher up in the type hierarchy),
/// methods on an interface that <paramref name="method"/>'s delcaring type implements,
/// methods on an interface that <paramref name="method"/>'s declaring type implements,
/// and methods an interface implemented by a derived type of <paramref name="method"/>'s declaring type if the derived type uses <paramref name="method"/> as the implementing method.
/// The list may be incomplete if there are derived types in assemblies that havent been processed yet that use <paramref name="method"/> to implement an interface.
/// </summary>
Expand Down
28 changes: 20 additions & 8 deletions src/tools/illink/src/linker/Linker/TypeMapInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
//

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

Expand All @@ -42,7 +43,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 +85,16 @@ public void EnsureProcessed (AssemblyDefinition assembly)
return bases;
}

public IEnumerable<(TypeDefinition InstanceType, InterfaceImplementation ProvidingInterface)>? GetDefaultInterfaceImplementations (MethodDefinition method)
/// <summary>
/// Returns a list of all default interface methods that implement <paramref name="method"/> for a type.
/// ImplementingType is the type that implements the interface,
/// InterfaceImpl is the <see cref="InterfaceImplementation" /> for the interface <paramref name="method" /> is declared on, and
/// 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 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 +118,15 @@ 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)
{
Debug.Assert(@base.DeclaringType.IsInterface);
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 @@ -278,6 +287,7 @@ void FindAndAddDefaultInterfaceImplementations (TypeDefinition type, MethodDefin
{
// Go over all interfaces, trying to find a method that is an explicit MethodImpl of the
// interface method in question.

foreach (var interfaceImpl in type.Interfaces) {
var potentialImplInterface = context.TryResolve (interfaceImpl.InterfaceType);
if (potentialImplInterface == null)
Expand All @@ -288,7 +298,9 @@ void FindAndAddDefaultInterfaceImplementations (TypeDefinition type, MethodDefin
foreach (var potentialImplMethod in potentialImplInterface.Methods) {
if (potentialImplMethod == interfaceMethod &&
!potentialImplMethod.IsAbstract) {
AddDefaultInterfaceImplementation (interfaceMethod, type, interfaceImpl);
AddDefaultInterfaceImplementation (interfaceMethod, type, (interfaceImpl, potentialImplMethod));
foundImpl = true;
break;
}

if (!potentialImplMethod.HasOverrides)
Expand All @@ -297,7 +309,7 @@ void FindAndAddDefaultInterfaceImplementations (TypeDefinition type, MethodDefin
// 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) {
AddDefaultInterfaceImplementation (interfaceMethod, type, interfaceImpl);
AddDefaultInterfaceImplementation (interfaceMethod, type, (interfaceImpl, @potentialImplMethod));
foundImpl = true;
break;
}
Expand Down
18 changes: 18 additions & 0 deletions ...Analyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.DefaultInterfaceMethodsTests.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,30 @@ 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 StaticDefaultInterfaceMethodOnStruct ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task UnusedDefaultInterfaceImplementation ()
{
Expand Down
Loading