-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Mark most specific static DIM for types marked RelevantToVariantCasting #97487
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
Merged
Merged
Changes from 7 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 642385d
Update Test infra
jtschuster 565837a
Mark static DIM if it provides an implementation for a type that is r…
jtschuster c3b0c63
Remove unused code and optimize default interface method marking
jtschuster e6d3788
Revert AssemblyChecker changes
jtschuster bb67ff1
Add example of overmarking in test
jtschuster 8b8a83d
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster 8851aae
wip
jtschuster e5b4a19
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster 4beb771
Keep all DIMs that provide an implementation for a kept interface method
jtschuster a46a642
Add generated tests
jtschuster 601658a
Fix test expectations
jtschuster 5520d85
Use ProcessDefaultImplementations for static iface methods
jtschuster 64e9f86
Undo unrelated changes
jtschuster 57ca7d1
Get rid of _interfaceOverrides, use Annotations.GetOverrides and Anno…
jtschuster c47c7d3
Clean up changes
jtschuster 694443a
Undo moving lines, update doc comments
jtschuster 1f0cdee
Merge branch 'main' into StaticDimFix
jtschuster 42d8764
Remove redundant test
jtschuster 386f644
Add test types to make sure all DIMs aren't kept
jtschuster cab167f
PR feedback
jtschuster bc4470f
Merge branch 'main' of https://github.com/dotnet/runtime into StaticD…
jtschuster 51be916
Break early if the DIM is the interface method
jtschuster File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -722,12 +722,12 @@ void ProcessVirtualMethod (MethodDefinition method) | |
| 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 (!Annotations.IsMarked (overrideInformation.Override.DeclaringType)) | ||
| return false; | ||
|
|
||
| if (!Context.IsOptimizationEnabled (CodeOptimizations.OverrideRemoval, overrideInformation.Override)) | ||
| return true; | ||
|
|
@@ -2275,9 +2275,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); | ||
| } | ||
|
|
@@ -2549,14 +2549,54 @@ 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 override is a DIM that provides an implementation for a type that requires the interface, we may need to mark the DIM | ||
jtschuster marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| var dims = Annotations.GetDefaultInterfaceImplementations (@base).Where(dim => Annotations.IsRelevantToVariantCasting (dim.InstanceType)); | ||
| if (dims.Any (dim => dim.DefaultInterfaceMethod == method && Annotations.IsRelevantToVariantCasting (dim.InstanceType))) | ||
jtschuster marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| // We need to find the most derived DIM for each type that has DIMs -- if there is a most derived DIM, we only mark that DIM | ||
jtschuster marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| var dimsByInstanceType = dims.GroupBy (dim => dim.InstanceType).Where(group => group.Any(dim => dim.DefaultInterfaceMethod == method)); | ||
| foreach (var group in dimsByInstanceType) { | ||
| var allDims = group.ToArray (); | ||
| int mostDerivedDimIndex = -1; | ||
| for (int i = 0; i < allDims.Length; i++) { | ||
| var derivesFromAllOtherDimProviders = true; | ||
| // Check if DIM i is the most specific DIM for the type by checking if it implements all the other DIM providers for the type | ||
| for (int j = 0; j < allDims.Length && derivesFromAllOtherDimProviders; j++) { | ||
| if (j == i) | ||
| continue; | ||
| // If the DIM provider i implements DIM provider j, then i is a more specific implementation that j. Otherwise, it is not and we check the next DIM provider | ||
| if (!allDims[i].DefaultInterfaceMethod.DeclaringType.Interfaces | ||
| .Any(iface => Context.Resolve(iface.InterfaceType) == allDims[j].DefaultInterfaceMethod.DeclaringType)) | ||
| { | ||
| derivesFromAllOtherDimProviders = false; | ||
| break; | ||
| } | ||
| } | ||
| if (derivesFromAllOtherDimProviders) { | ||
| mostDerivedDimIndex = i; | ||
| break; | ||
| } | ||
| } | ||
| // If there is a most derived DIM, we only need to mark that DIM -- return true if the override is the most derived DIM | ||
| if (mostDerivedDimIndex != -1) { | ||
| if (allDims[mostDerivedDimIndex].DefaultInterfaceMethod == method) | ||
| return true; | ||
| else | ||
| continue; | ||
| } else { | ||
| // If there is no most derived DIM, all DIMs should be marked. | ||
| // We already checked that the override is a DIM that provides an implementation for a type that requires the interface, so we can return true | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 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. | ||
|
|
@@ -3007,7 +3047,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)); | ||
|
|
@@ -3178,14 +3218,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)); | ||
|
|
@@ -3231,7 +3272,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 ()) { | ||
|
|
@@ -3315,7 +3356,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 | ||
|
|
@@ -3335,11 +3376,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; | ||
|
|
@@ -3584,7 +3625,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); | ||
|
|
@@ -3762,8 +3803,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; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
...s/Inheritance.Interfaces/DefaultInterfaceMethods/MostSpecificDefaultImplementationKept.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| 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 MostSpecificDefaultImplementationKept | ||
| { | ||
| [Kept] | ||
| public static void Main () | ||
| { | ||
| #if SUPPORTS_DEFAULT_INTERFACE_METHODS | ||
| M<UsedAsIBase> (); | ||
| NotUsedInGeneric.Keep (); | ||
| GenericType<UsedAsIBase2>.M (); | ||
| GenericType2<UsedInUnconstrainedGeneric>.Keep (); | ||
| #endif | ||
| } | ||
|
|
||
| #if SUPPORTS_DEFAULT_INTERFACE_METHODS | ||
|
|
||
| [Kept] | ||
| static int M<T> () where T : IBase | ||
| { | ||
| return T.Value; | ||
| } | ||
|
|
||
| [Kept] | ||
| interface IBase | ||
| { | ||
| [Kept] | ||
| static virtual int Value { | ||
| [Kept] | ||
| get => 0; | ||
| } | ||
|
|
||
| static virtual int Value2 { | ||
| get => 0; | ||
| } | ||
| } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IBase))] | ||
| interface IMiddle : IBase | ||
| { | ||
| static int IBase.Value { | ||
| get => 1; | ||
| } | ||
| } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IBase))] | ||
| [KeptInterface (typeof (IMiddle))] | ||
| interface IDerived : IMiddle | ||
| { | ||
| [Kept] | ||
| static int IBase.Value { | ||
| [Kept] | ||
| get => 2; | ||
| } | ||
| } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IBase))] | ||
| [KeptInterface (typeof (IMiddle))] | ||
| interface IDerived2 : IMiddle | ||
| { | ||
| // https://github.com/dotnet/runtime/issues/97798 | ||
| // This shouldn't need to be kept. Implementor UsedInUnconstrainedGeneric is not passed as a constrained generic | ||
| [Kept] | ||
| static int IBase.Value { | ||
| [Kept] | ||
| get => 2; | ||
| } | ||
| } | ||
|
|
||
| interface INotReferenced | ||
| { } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IDerived))] | ||
| [KeptInterface (typeof (IMiddle))] | ||
| [KeptInterface (typeof (IBase))] | ||
| class UsedAsIBase : IDerived, INotReferenced | ||
| { | ||
| } | ||
|
|
||
| [Kept] | ||
| class NotUsedInGeneric : IDerived, INotReferenced | ||
| { | ||
| [Kept] | ||
| public static void Keep () { } | ||
| } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IBase))] | ||
| [KeptInterface (typeof (IMiddle))] | ||
| [KeptInterface (typeof (IDerived2))] | ||
| class UsedInUnconstrainedGeneric : IDerived2, INotReferenced | ||
| { | ||
| } | ||
|
|
||
|
|
||
| [Kept] | ||
| class GenericType<T> where T : IBase | ||
| { | ||
| [Kept] | ||
| public static int M () => T.Value; | ||
| } | ||
|
|
||
| [Kept] | ||
| class GenericType2<T> | ||
| { | ||
| [Kept] | ||
| public static void Keep() { } | ||
| } | ||
|
|
||
| [Kept] | ||
| [KeptInterface (typeof (IDerived))] | ||
| [KeptInterface (typeof (IMiddle))] | ||
| [KeptInterface (typeof (IBase))] | ||
| class UsedAsIBase2 : IDerived | ||
| { | ||
| } | ||
| #endif | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.