From 0388e488a96d73dfa3330eff1123b488fd4b16ce Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 31 Aug 2021 08:22:40 -0700 Subject: [PATCH 01/13] Consider all public types in a library as instantiated This will make sure that we keep all interfaces and all interface method implementations on such type. --- src/linker/Linker.Steps/MarkStep.cs | 3 +++ .../Libraries/RootLibrary.cs | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 90deb75047b7..297b4a99fb45 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2626,6 +2626,9 @@ void ApplyPreserveInfo (TypeDefinition type) } } } + + if (members.HasFlag (TypePreserveMembers.Library)) + MarkRequirementsForInstantiatedTypes (type); } } diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index e9e7ecf29ee6..a8d5524c7330 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.Serialization; @@ -173,6 +175,22 @@ public override string ToString () public interface I { } + + [Kept] + [KeptInterface (typeof (IEnumerator))] + public class UninstantiatedPublicClassWithInterface : IEnumerator + { + internal UninstantiatedPublicClassWithInterface () { } + + [Kept] + bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); } + + [Kept] + object IEnumerator.Current { [Kept] get { throw new PlatformNotSupportedException (); } } + + [Kept] + void IEnumerator.Reset () {} + } } internal class RootLibrary_Internal From 39ffb9f081f0a23eaf2aeb3ce532a1695a502ca2 Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 31 Aug 2021 13:51:26 -0700 Subject: [PATCH 02/13] Change the solution to rely on the optimization setting only --- src/linker/Linker.Steps/MarkStep.cs | 44 +++++++++++++++++-- .../InterfaceOnUninstantiatedTypeRemoved.cs | 1 - .../Libraries/RootLibrary.cs | 27 ++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 297b4a99fb45..4dc831b0588a 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -581,11 +581,27 @@ void ProcessMarkedTypesWithInterfaces () foreach ((var type, var scope) in typesWithInterfaces) { // Exception, types that have not been flagged as instantiated yet. These types may not need their interfaces even if the // interface type is marked - if (!Annotations.IsInstantiated (type) && !Annotations.IsRelevantToVariantCasting (type)) + // UnusedInterfaces optimization is turned off mark all interface implementations + bool unusedInterfacesOptimizationEnabled = _context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, type); + if (!Annotations.IsInstantiated (type) && !Annotations.IsRelevantToVariantCasting (type) && + unusedInterfacesOptimizationEnabled) continue; using (ScopeStack.PushScope (scope)) MarkInterfaceImplementations (type); + + if (unusedInterfacesOptimizationEnabled) + continue; + + // If the optimization is disabled, make sure to mark all methods which implement interfaces + foreach (MethodDefinition method in type.Methods) { + if (!IsMethodNeededByTypeToImplementInterface (method)) + continue; + + if (!Annotations.IsMarked (method)) + MarkMethod (method, new DependencyInfo (DependencyKind.MethodForInstantiatedType, type)); + } + } } } @@ -2326,6 +2342,29 @@ bool IsVirtualNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition meth return false; } + bool IsMethodNeededByTypeToImplementInterface (MethodDefinition method) + { + if (!method.IsVirtual) + return false; + + var base_list = Annotations.GetBaseMethods (method); + if (base_list == null) + return false; + + foreach (MethodDefinition @base in base_list) { + if (!@base.DeclaringType.IsInterface) + continue; + + if (IgnoreScope (@base.DeclaringType.Scope)) + return true; + + if (IsVirtualNeededByTypeDueToPreservedScope (@base)) + return true; + } + + return false; + } + static bool IsSpecialSerializationConstructor (MethodDefinition method) { if (!method.IsInstanceConstructor ()) @@ -2626,9 +2665,6 @@ void ApplyPreserveInfo (TypeDefinition type) } } } - - if (members.HasFlag (TypePreserveMembers.Library)) - MarkRequirementsForInstantiatedTypes (type); } } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceOnUninstantiatedTypeRemoved.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceOnUninstantiatedTypeRemoved.cs index 6b2cbd51104c..1c4a08112c3e 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceOnUninstantiatedTypeRemoved.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceOnUninstantiatedTypeRemoved.cs @@ -3,7 +3,6 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces { - [SetupLinkerArgument ("--disable-opt", "unusedinterfaces")] public class InterfaceOnUninstantiatedTypeRemoved { public static void Main () diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index a8d5524c7330..fe4cee440e44 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -191,6 +191,33 @@ internal UninstantiatedPublicClassWithInterface () { } [Kept] void IEnumerator.Reset () {} } + + [Kept] + [KeptInterface (typeof (IPublicInterface))] + [KeptInterface (typeof (IInternalInterface))] + public class InstantiatedClassWithInterfaces : IPublicInterface, IInternalInterface + { + [Kept] + public InstantiatedClassWithInterfaces () { } + + [Kept] + void IPublicInterface.PublicInterfaceMethod () { } + + void IInternalInterface.InternalInterfaceMethod () { } + } + + [Kept] + public interface IPublicInterface + { + [Kept] + void PublicInterfaceMethod (); + } + + [Kept] + internal interface IInternalInterface + { + void InternalInterfaceMethod (); + } } internal class RootLibrary_Internal From e5b37884ca99b7da0d256b38f1a9ed73c5e61dde Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 25 Mar 2022 14:37:02 -0700 Subject: [PATCH 03/13] wip --- src/linker/Linker.Steps/MarkStep.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 4dc831b0588a..e68a248c053a 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -582,12 +582,12 @@ void ProcessMarkedTypesWithInterfaces () // Exception, types that have not been flagged as instantiated yet. These types may not need their interfaces even if the // interface type is marked // UnusedInterfaces optimization is turned off mark all interface implementations - bool unusedInterfacesOptimizationEnabled = _context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, type); + bool unusedInterfacesOptimizationEnabled = Context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, type); if (!Annotations.IsInstantiated (type) && !Annotations.IsRelevantToVariantCasting (type) && unusedInterfacesOptimizationEnabled) continue; - using (ScopeStack.PushScope (scope)) + using (ScopeStack.PushScope (scope)) { MarkInterfaceImplementations (type); if (unusedInterfacesOptimizationEnabled) From 61115f616112f2b7112bf5547c2a2fb6f92931ab Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 28 Mar 2022 11:50:17 -0700 Subject: [PATCH 04/13] Add implicit interface implementation case --- .../Libraries/RootLibrary.cs | 156 +++++++++++++++++- 1 file changed, 155 insertions(+), 1 deletion(-) diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index fe4cee440e44..29fc33809365 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -1,6 +1,8 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.Serialization; using Mono.Linker.Tests.Cases.Expectations.Assertions; @@ -10,6 +12,7 @@ namespace Mono.Linker.Tests.Cases.Libraries { [SetupLinkerArgument ("-a", "test.exe", "library")] [SetupLinkerArgument ("--enable-opt", "ipconstprop")] + [SetupLinkerArgument ("--skip-unresolved")] [VerifyMetadataNames] public class RootLibrary { @@ -189,7 +192,72 @@ internal UninstantiatedPublicClassWithInterface () { } object IEnumerator.Current { [Kept] get { throw new PlatformNotSupportedException (); } } [Kept] - void IEnumerator.Reset () {} + void IEnumerator.Reset () { } + } + + [Kept] + [KeptInterface (typeof (ICollection))] + [KeptInterface (typeof (IEnumerable))] + [KeptInterface (typeof (IEnumerable))] + public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : ICollection + { + [Kept] + [KeptBackingField] + public int Count { [Kept] get; } + + [Kept] + [KeptBackingField] + public bool IsReadOnly { [Kept] get; } + + internal UninstantiatedPublicClassWithImplicitlyImplementedInterface () { } + + [Kept] + public void CopyTo (Array array, int index) + { + throw new NotImplementedException (); + } + + [Kept] + public void Add (int item) + { + throw new NotImplementedException (); + } + + [Kept] + public void Clear () + { + throw new NotImplementedException (); + } + + [Kept] + public bool Contains (int item) + { + throw new NotImplementedException (); + } + + [Kept] + public void CopyTo (int[] array, int arrayIndex) + { + throw new NotImplementedException (); + } + + [Kept] + public bool Remove (int item) + { + throw new NotImplementedException (); + } + + [Kept] + public IEnumerator GetEnumerator () + { + throw new NotImplementedException (); + } + + [Kept] + IEnumerator IEnumerable.GetEnumerator () + { + throw new NotImplementedException (); + } } [Kept] @@ -230,4 +298,90 @@ internal void Unused () { } } + + [Kept] + [KeptInterface (typeof (ICollection))] + [KeptInterface (typeof (IEnumerable))] + [KeptInterface (typeof (IEnumerable))] + [KeptAttributeAttribute (typeof (System.Reflection.DefaultMemberAttribute))] + public class IPAddressInformationCollection : ICollection + { + [Kept] + private readonly List _addresses = new List (); + + internal IPAddressInformationCollection () + { + } + + [Kept] + public virtual void CopyTo (IPAddressInformation[] array, int offset) + { + _addresses.CopyTo (array, offset); + } + + [Kept] + public virtual int Count { + [Kept] + get { + return _addresses.Count; + } + } + + [Kept] + public virtual bool IsReadOnly { + [Kept] + get { + return true; + } + } + + [Kept] + public virtual void Add (IPAddressInformation address) + { + throw new NotSupportedException (); + } + + internal void InternalAdd (IPAddressInformation address) + { + _addresses.Add (address); + } + + [Kept] + public virtual bool Contains (IPAddressInformation address) + { + return _addresses.Contains (address); + } + + [Kept] + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator () + { return this.GetEnumerator (); } + [Kept] + public virtual IEnumerator GetEnumerator () { return _addresses.GetEnumerator (); } + [Kept] + public virtual IPAddressInformation this[int index] { + [Kept] + get { + return _addresses[index]; + } + } + + [Kept] + public virtual bool Remove (IPAddressInformation address) + { + throw new NotSupportedException (); + } + + [Kept] + public virtual void Clear () + { + throw new NotSupportedException (); + } + } + [Kept] + public class IPAddressInformation + { + [Kept] + public IPAddressInformation () { } + + } } From c4ebd45d90662ba5312edc74762018aa64b6afd9 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 28 Mar 2022 12:12:29 -0700 Subject: [PATCH 05/13] Edit tests to remove issue-specific code and names --- .../Libraries/RootLibrary.cs | 100 ++---------------- 1 file changed, 8 insertions(+), 92 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index 29fc33809365..67922972e58b 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -199,7 +199,7 @@ void IEnumerator.Reset () { } [KeptInterface (typeof (ICollection))] [KeptInterface (typeof (IEnumerable))] [KeptInterface (typeof (IEnumerable))] - public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : ICollection + public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : ICollection { [Kept] [KeptBackingField] @@ -218,7 +218,7 @@ public void CopyTo (Array array, int index) } [Kept] - public void Add (int item) + public void Add (CollectedType item) { throw new NotImplementedException (); } @@ -230,19 +230,19 @@ public void Clear () } [Kept] - public bool Contains (int item) + public bool Contains (CollectedType item) { throw new NotImplementedException (); } [Kept] - public void CopyTo (int[] array, int arrayIndex) + public void CopyTo (CollectedType[] array, int arrayIndex) { throw new NotImplementedException (); } [Kept] - public bool Remove (int item) + public bool Remove (CollectedType item) { throw new NotImplementedException (); } @@ -254,12 +254,14 @@ public IEnumerator GetEnumerator () } [Kept] - IEnumerator IEnumerable.GetEnumerator () + IEnumerator IEnumerable.GetEnumerator () { throw new NotImplementedException (); } } + public class CollectedType { } + [Kept] [KeptInterface (typeof (IPublicInterface))] [KeptInterface (typeof (IInternalInterface))] @@ -298,90 +300,4 @@ internal void Unused () { } } - - [Kept] - [KeptInterface (typeof (ICollection))] - [KeptInterface (typeof (IEnumerable))] - [KeptInterface (typeof (IEnumerable))] - [KeptAttributeAttribute (typeof (System.Reflection.DefaultMemberAttribute))] - public class IPAddressInformationCollection : ICollection - { - [Kept] - private readonly List _addresses = new List (); - - internal IPAddressInformationCollection () - { - } - - [Kept] - public virtual void CopyTo (IPAddressInformation[] array, int offset) - { - _addresses.CopyTo (array, offset); - } - - [Kept] - public virtual int Count { - [Kept] - get { - return _addresses.Count; - } - } - - [Kept] - public virtual bool IsReadOnly { - [Kept] - get { - return true; - } - } - - [Kept] - public virtual void Add (IPAddressInformation address) - { - throw new NotSupportedException (); - } - - internal void InternalAdd (IPAddressInformation address) - { - _addresses.Add (address); - } - - [Kept] - public virtual bool Contains (IPAddressInformation address) - { - return _addresses.Contains (address); - } - - [Kept] - System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator () - { return this.GetEnumerator (); } - [Kept] - public virtual IEnumerator GetEnumerator () { return _addresses.GetEnumerator (); } - [Kept] - public virtual IPAddressInformation this[int index] { - [Kept] - get { - return _addresses[index]; - } - } - - [Kept] - public virtual bool Remove (IPAddressInformation address) - { - throw new NotSupportedException (); - } - - [Kept] - public virtual void Clear () - { - throw new NotSupportedException (); - } - } - [Kept] - public class IPAddressInformation - { - [Kept] - public IPAddressInformation () { } - - } } From d7cc881aed2b88ae2df0e89281a9684915571157 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 28 Mar 2022 14:12:55 -0700 Subject: [PATCH 06/13] Mark members of CollectedType as kept --- test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index 67922972e58b..a9333839bd8b 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -196,8 +196,8 @@ void IEnumerator.Reset () { } } [Kept] - [KeptInterface (typeof (ICollection))] - [KeptInterface (typeof (IEnumerable))] + [KeptInterface (typeof (ICollection))] + [KeptInterface (typeof (IEnumerable))] [KeptInterface (typeof (IEnumerable))] public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : ICollection { @@ -260,6 +260,8 @@ IEnumerator IEnumerable.GetEnumerator () } } + [Kept] + [KeptMember (".ctor()")] public class CollectedType { } [Kept] From 3a8b76dd38e5be9c8521dd43dc210a9a61b5ff6e Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Thu, 31 Mar 2022 13:14:58 -0700 Subject: [PATCH 07/13] Add private interface test and simplify external interface example --- src/linker/Linker.Steps/MarkStep.cs | 2 - .../Libraries/RootLibrary.cs | 80 +++++-------------- 2 files changed, 21 insertions(+), 61 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index e68a248c053a..135493d1b699 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2352,8 +2352,6 @@ bool IsMethodNeededByTypeToImplementInterface (MethodDefinition method) return false; foreach (MethodDefinition @base in base_list) { - if (!@base.DeclaringType.IsInterface) - continue; if (IgnoreScope (@base.DeclaringType.Scope)) return true; diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index a9333839bd8b..d86b9085a338 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -12,7 +12,6 @@ namespace Mono.Linker.Tests.Cases.Libraries { [SetupLinkerArgument ("-a", "test.exe", "library")] [SetupLinkerArgument ("--enable-opt", "ipconstprop")] - [SetupLinkerArgument ("--skip-unresolved")] [VerifyMetadataNames] public class RootLibrary { @@ -196,74 +195,22 @@ void IEnumerator.Reset () { } } [Kept] - [KeptInterface (typeof (ICollection))] - [KeptInterface (typeof (IEnumerable))] - [KeptInterface (typeof (IEnumerable))] - public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : ICollection + [KeptInterface (typeof (IInternalInterface))] + [KeptInterface (typeof (IFormattable))] + public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : IInternalInterface, IFormattable { - [Kept] - [KeptBackingField] - public int Count { [Kept] get; } - - [Kept] - [KeptBackingField] - public bool IsReadOnly { [Kept] get; } - internal UninstantiatedPublicClassWithImplicitlyImplementedInterface () { } [Kept] - public void CopyTo (Array array, int index) - { - throw new NotImplementedException (); - } - - [Kept] - public void Add (CollectedType item) - { - throw new NotImplementedException (); - } - - [Kept] - public void Clear () - { - throw new NotImplementedException (); - } - - [Kept] - public bool Contains (CollectedType item) - { - throw new NotImplementedException (); - } - - [Kept] - public void CopyTo (CollectedType[] array, int arrayIndex) - { - throw new NotImplementedException (); - } - - [Kept] - public bool Remove (CollectedType item) - { - throw new NotImplementedException (); - } + public void InternalInterfaceMethod () { } [Kept] - public IEnumerator GetEnumerator () + public string ToString (string format, IFormatProvider formatProvider) { - throw new NotImplementedException (); - } - - [Kept] - IEnumerator IEnumerable.GetEnumerator () - { - throw new NotImplementedException (); + return "formatted string"; } } - [Kept] - [KeptMember (".ctor()")] - public class CollectedType { } - [Kept] [KeptInterface (typeof (IPublicInterface))] [KeptInterface (typeof (IInternalInterface))] @@ -278,6 +225,15 @@ void IPublicInterface.PublicInterfaceMethod () { } void IInternalInterface.InternalInterfaceMethod () { } } + [Kept] + [KeptInterface (typeof (IPrivateInterface))] + public class UninstantiatedPublicClassWithPrivateInterface : IPrivateInterface + { + internal UninstantiatedPublicClassWithPrivateInterface () { } + + void IPrivateInterface.PrivateInterfaceMethod () { } + } + [Kept] public interface IPublicInterface { @@ -290,6 +246,12 @@ internal interface IInternalInterface { void InternalInterfaceMethod (); } + + [Kept] + private interface IPrivateInterface + { + void PrivateInterfaceMethod (); + } } internal class RootLibrary_Internal From 1c04ad2389ea3336050a419bf495b38671a1cb32 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 1 Apr 2022 13:49:19 -0700 Subject: [PATCH 08/13] Replace early exit for non-interfaces --- src/linker/Linker.Steps/MarkStep.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 135493d1b699..e68a248c053a 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2352,6 +2352,8 @@ bool IsMethodNeededByTypeToImplementInterface (MethodDefinition method) return false; foreach (MethodDefinition @base in base_list) { + if (!@base.DeclaringType.IsInterface) + continue; if (IgnoreScope (@base.DeclaringType.Scope)) return true; From 7ad5afcc992a6f33af15643bc1dc58533683295b Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Fri, 1 Apr 2022 14:30:07 -0700 Subject: [PATCH 09/13] License headers and use IsMethodNeededByTypeDueToPreservedScope instead of Interface specific version --- src/linker/Linker.Dataflow/ArrayValue.cs | 4 +-- src/linker/Linker.Dataflow/FieldValue.cs | 4 +-- .../Linker.Dataflow/GenericParameterValue.cs | 4 +-- .../Linker.Dataflow/MethodParameterValue.cs | 4 +-- .../Linker.Dataflow/MethodReturnValue.cs | 4 +-- .../MethodThisParameterValue.cs | 4 +-- .../RuntimeMethodHandleValue.cs | 4 +-- .../Linker.Dataflow/SingleValueExtensions.cs | 4 +-- src/linker/Linker.Steps/MarkStep.cs | 29 ++----------------- 9 files changed, 19 insertions(+), 42 deletions(-) diff --git a/src/linker/Linker.Dataflow/ArrayValue.cs b/src/linker/Linker.Dataflow/ArrayValue.cs index 884e5cd7e0cc..71934bc28ca8 100644 --- a/src/linker/Linker.Dataflow/ArrayValue.cs +++ b/src/linker/Linker.Dataflow/ArrayValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Generic; diff --git a/src/linker/Linker.Dataflow/FieldValue.cs b/src/linker/Linker.Dataflow/FieldValue.cs index 41631512815e..6d20f8301c79 100644 --- a/src/linker/Linker.Dataflow/FieldValue.cs +++ b/src/linker/Linker.Dataflow/FieldValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; diff --git a/src/linker/Linker.Dataflow/GenericParameterValue.cs b/src/linker/Linker.Dataflow/GenericParameterValue.cs index 7f6e104b13c4..f96ae719eac6 100644 --- a/src/linker/Linker.Dataflow/GenericParameterValue.cs +++ b/src/linker/Linker.Dataflow/GenericParameterValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; diff --git a/src/linker/Linker.Dataflow/MethodParameterValue.cs b/src/linker/Linker.Dataflow/MethodParameterValue.cs index 02085722312a..325b02852739 100644 --- a/src/linker/Linker.Dataflow/MethodParameterValue.cs +++ b/src/linker/Linker.Dataflow/MethodParameterValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; diff --git a/src/linker/Linker.Dataflow/MethodReturnValue.cs b/src/linker/Linker.Dataflow/MethodReturnValue.cs index 10414a670940..f6cb423c121e 100644 --- a/src/linker/Linker.Dataflow/MethodReturnValue.cs +++ b/src/linker/Linker.Dataflow/MethodReturnValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; diff --git a/src/linker/Linker.Dataflow/MethodThisParameterValue.cs b/src/linker/Linker.Dataflow/MethodThisParameterValue.cs index e9ded179ba2e..c2f46d5996e1 100644 --- a/src/linker/Linker.Dataflow/MethodThisParameterValue.cs +++ b/src/linker/Linker.Dataflow/MethodThisParameterValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; diff --git a/src/linker/Linker.Dataflow/RuntimeMethodHandleValue.cs b/src/linker/Linker.Dataflow/RuntimeMethodHandleValue.cs index 7197b5491a43..c90ecef4bbb9 100644 --- a/src/linker/Linker.Dataflow/RuntimeMethodHandleValue.cs +++ b/src/linker/Linker.Dataflow/RuntimeMethodHandleValue.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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 ILLink.Shared.DataFlow; using Mono.Cecil; diff --git a/src/linker/Linker.Dataflow/SingleValueExtensions.cs b/src/linker/Linker.Dataflow/SingleValueExtensions.cs index b136c9fef5ca..c0024c1ccc13 100644 --- a/src/linker/Linker.Dataflow/SingleValueExtensions.cs +++ b/src/linker/Linker.Dataflow/SingleValueExtensions.cs @@ -1,5 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// 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.Generic; diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index d8519d487a76..97f74c6a628d 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -598,7 +598,7 @@ void ProcessMarkedTypesWithInterfaces () // If the optimization is disabled, make sure to mark all methods which implement interfaces foreach (MethodDefinition method in type.Methods) { - if (!IsMethodNeededByTypeToImplementInterface (method)) + if (!IsMethodNeededByTypeDueToPreservedScope (method)) continue; if (!Annotations.IsMarked (method)) @@ -2325,7 +2325,7 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) return false; } - bool IsVirtualNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) + bool IsMethodNeededByTypeDueToPreservedScope (MethodDefinition method) { if (!method.IsVirtual) return false; @@ -2345,29 +2345,6 @@ bool IsVirtualNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition meth return false; } - bool IsMethodNeededByTypeToImplementInterface (MethodDefinition method) - { - if (!method.IsVirtual) - return false; - - var base_list = Annotations.GetBaseMethods (method); - if (base_list == null) - return false; - - foreach (MethodDefinition @base in base_list) { - if (!@base.DeclaringType.IsInterface) - continue; - - if (IgnoreScope (@base.DeclaringType.Scope)) - return true; - - if (IsVirtualNeededByTypeDueToPreservedScope (@base)) - return true; - } - - return false; - } - static bool IsSpecialSerializationConstructor (MethodDefinition method) { if (!method.IsInstanceConstructor ()) @@ -3149,7 +3126,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type protected virtual IEnumerable GetRequiredMethodsForInstantiatedType (TypeDefinition type) { foreach (var method in type.Methods) { - if (IsVirtualNeededByInstantiatedTypeDueToPreservedScope (method)) + if (IsMethodNeededByTypeDueToPreservedScope (method)) yield return method; } } From 07143ddb394e32aa163cc3420c7cbbf81f811e24 Mon Sep 17 00:00:00 2001 From: Jackson Schuster Date: Mon, 4 Apr 2022 17:38:54 -0700 Subject: [PATCH 10/13] Add check for static methods before skipping virtual marking Check for optimization before skipping marking interface methods for PreservedScope Add doc comments --- src/linker/Linker.Steps/MarkStep.cs | 36 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 97f74c6a628d..07c942521bdb 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -592,18 +592,6 @@ void ProcessMarkedTypesWithInterfaces () using (ScopeStack.PushScope (scope)) { MarkInterfaceImplementations (type); - - if (unusedInterfacesOptimizationEnabled) - continue; - - // If the optimization is disabled, make sure to mark all methods which implement interfaces - foreach (MethodDefinition method in type.Methods) { - if (!IsMethodNeededByTypeDueToPreservedScope (method)) - continue; - - if (!Annotations.IsMarked (method)) - MarkMethod (method, new DependencyInfo (DependencyKind.MethodForInstantiatedType, type)); - } } } } @@ -1735,6 +1723,9 @@ void MarkField (FieldDefinition field, in DependencyInfo reason) } } + /// + /// Returns true if the assembly of the is not set to link (i.e. action=copy is set for that assembly) + /// protected virtual bool IgnoreScope (IMetadataScope scope) { AssemblyDefinition? assembly = Context.Resolve (scope); @@ -1931,7 +1922,7 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in MarkGenericParameterProvider (type); // There are a number of markings we can defer until later when we know it's possible a reference type could be instantiated - // For example, if no instance of a type exist, then we don't need to mark the interfaces on that type + // For example, if no instance of a type exist, then we don't need to mark the interfaces on that type -- Note this is not true for static interfaces // However, for some other types there is no benefit to deferring if (type.IsInterface) { // There's no benefit to deferring processing of an interface type until we know a type implementing that interface is marked @@ -1955,6 +1946,7 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in MarkRequirementsForInstantiatedTypes (type); } + // Save for later once we know which interfaces are marked and then determine which interface implementations and methods to keep if (type.HasInterfaces) _typesWithInterfaces.Add ((type, ScopeStack.CurrentScope)); @@ -2294,6 +2286,10 @@ void MarkGenericParameter (GenericParameter parameter) } } + /// + /// Returns true if any of the base methods of the passed is in an assembly that is not trimmed (i.e. action != trim) + /// + /// This ignores any base methods defined in interfaces. To also check methods defined in interfaces bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) { if (!method.IsVirtual) @@ -2305,8 +2301,8 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) foreach (MethodDefinition @base in base_list) { // Just because the type is marked does not mean we need interface methods. - // if the type is never instantiated, interfaces will be removed - if (@base.DeclaringType.IsInterface) + // if the type is never instantiated, interfaces will be removed - but only if the optimization is enabled + if (@base.DeclaringType.IsInterface && Context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, method.DeclaringType)) continue; // If the type is marked, we need to keep overrides of abstract members defined in assemblies @@ -2325,9 +2321,13 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) return false; } - bool IsMethodNeededByTypeDueToPreservedScope (MethodDefinition method) + /// + /// Returns true if any of the base methods of is defined in an assembly that is not trimmed (i.e. action!=trim) + /// + /// This is very similar to , except this also checks if the base method is an interface. + bool IsVirtualNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) { - if (!method.IsVirtual) + if (!(method.IsVirtual || method.IsStatic)) return false; var base_list = Annotations.GetBaseMethods (method); @@ -3126,7 +3126,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type protected virtual IEnumerable GetRequiredMethodsForInstantiatedType (TypeDefinition type) { foreach (var method in type.Methods) { - if (IsMethodNeededByTypeDueToPreservedScope (method)) + if (IsVirtualNeededByInstantiatedTypeDueToPreservedScope (method)) yield return method; } } From a7c7e13e9cb598a290ebd0c1f93b27c44ac2b05d Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 6 Apr 2022 22:57:20 +0000 Subject: [PATCH 11/13] Add more clarifying comments, move static method check, rename method Rename IsVirtualNeededByInstantiatedTypeDueToPreservedScope to IsOverrideNeededByInstantiatedTypeDueToPreservedScope Added more comments describing purpose of methods Moved the static interface method check to the method that is called on all types regardless of instantiation --- src/linker/Linker.Steps/MarkStep.cs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 07c942521bdb..da8f245e82d3 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2287,12 +2287,18 @@ void MarkGenericParameter (GenericParameter parameter) } /// - /// Returns true if any of the base methods of the passed is in an assembly that is not trimmed (i.e. action != trim) + /// Returns true if any of the base methods of the passed is in an assembly that is not trimmed (i.e. action != trim). + /// Meant to be used to determine whether methods should be marked regardless of whether it is instantiated or not. /// - /// This ignores any base methods defined in interfaces. To also check methods defined in interfaces + /// + /// When the unusedinterfaces optimization is on, this is used to mark methods that override a virtual from a non-link assembly and must be kept. + /// When the unusedinterfaces optimization is off, this will do the same as when on but will also mark interface methods from interfaces defined in a non-link assembly. + /// If the containing type is instantiated, the caller should use + /// bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) { - if (!method.IsVirtual) + // Static methods may also have base methods in static interface methods. These methods are not captured by IsVirtual and must be checked separately + if (!(method.IsVirtual || method.IsStatic)) return false; var base_list = Annotations.GetBaseMethods (method); @@ -2322,12 +2328,17 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) } /// - /// Returns true if any of the base methods of is defined in an assembly that is not trimmed (i.e. action!=trim) + /// Returns true if any of the base methods of is defined in an assembly that is not trimmed (i.e. action!=trim). + /// This is meant to be used on methods from a type that is known to be instantiated. /// - /// This is very similar to , except this also checks if the base method is an interface. - bool IsVirtualNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) + /// + /// This is very similar to , + /// but will mark methods from an interface defined in a non-link assembly regardless of the optimization, and does not handle static interface methods. + /// + bool IsOverrideNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) { - if (!(method.IsVirtual || method.IsStatic)) + // Any static interface methods are captured by , which should be called on all relevant methods so no need to check again here. + if (!method.IsVirtual) return false; var base_list = Annotations.GetBaseMethods (method); @@ -3126,7 +3137,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type protected virtual IEnumerable GetRequiredMethodsForInstantiatedType (TypeDefinition type) { foreach (var method in type.Methods) { - if (IsVirtualNeededByInstantiatedTypeDueToPreservedScope (method)) + if (IsOverrideNeededByInstantiatedTypeDueToPreservedScope (method)) yield return method; } } From 64692a0fad48f44c3fde215d10c1b1db5453c25d Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 7 Apr 2022 07:27:42 -0700 Subject: [PATCH 12/13] Renames and comment cleanup + tests This doesn't change product behavior, only renamed methods and improved comments. Added new tests for the RootLibrary: - Added a dependency to an "copy" assembly (mainly because I can define a static interface in it) - Added more combinations to the interfaces/classes in the test - Since this uses static interface methods I had to enable "preview" language features for the test project and for the test infra. --- Directory.Build.props | 2 +- src/linker/Linker.Steps/MarkStep.cs | 20 +-- .../Libraries/Dependencies/CopyLibrary.cs | 23 +++ .../Libraries/RootLibrary.cs | 134 +++++++++++++++++- .../TestCasesRunner/TestCaseCompiler.cs | 2 +- 5 files changed, 165 insertions(+), 16 deletions(-) create mode 100644 test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs diff --git a/Directory.Build.props b/Directory.Build.props index c6b56b26ad11..ed94e7f27595 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -29,7 +29,7 @@ - latest + preview latest diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index da8f245e82d3..2c9c7e0386f2 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -1951,8 +1951,8 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in _typesWithInterfaces.Add ((type, ScopeStack.CurrentScope)); if (type.HasMethods) { - // For virtuals that must be preserved, blame the declaring type. - MarkMethodsIf (type.Methods, IsVirtualNeededByTypeDueToPreservedScope, new DependencyInfo (DependencyKind.VirtualNeededDueToPreservedScope, type)); + // For methods that must be preserved, blame the declaring type. + MarkMethodsIf (type.Methods, IsMethodNeededByTypeDueToPreservedScope, new DependencyInfo (DependencyKind.VirtualNeededDueToPreservedScope, type)); if (ShouldMarkTypeStaticConstructor (type) && reason.Kind != DependencyKind.TriggersCctorForCalledMethod) { using (ScopeStack.PopToParent ()) MarkStaticConstructor (type, new DependencyInfo (DependencyKind.CctorForType, type)); @@ -2291,11 +2291,11 @@ void MarkGenericParameter (GenericParameter parameter) /// Meant to be used to determine whether methods should be marked regardless of whether it is instantiated or not. /// /// - /// When the unusedinterfaces optimization is on, this is used to mark methods that override a virtual from a non-link assembly and must be kept. + /// When the unusedinterfaces optimization is on, this is used to mark methods that override an abstract method from a non-link assembly and must be kept. /// When the unusedinterfaces optimization is off, this will do the same as when on but will also mark interface methods from interfaces defined in a non-link assembly. - /// If the containing type is instantiated, the caller should use + /// If the containing type is instantiated, the caller should also use /// - bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) + bool IsMethodNeededByTypeDueToPreservedScope (MethodDefinition method) { // Static methods may also have base methods in static interface methods. These methods are not captured by IsVirtual and must be checked separately if (!(method.IsVirtual || method.IsStatic)) @@ -2320,7 +2320,7 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) if (IgnoreScope (@base.DeclaringType.Scope)) return true; - if (IsVirtualNeededByTypeDueToPreservedScope (@base)) + if (IsMethodNeededByTypeDueToPreservedScope (@base)) return true; } @@ -2332,10 +2332,10 @@ bool IsVirtualNeededByTypeDueToPreservedScope (MethodDefinition method) /// This is meant to be used on methods from a type that is known to be instantiated. /// /// - /// This is very similar to , + /// This is very similar to , /// but will mark methods from an interface defined in a non-link assembly regardless of the optimization, and does not handle static interface methods. /// - bool IsOverrideNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) + bool IsMethodNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition method) { // Any static interface methods are captured by , which should be called on all relevant methods so no need to check again here. if (!method.IsVirtual) @@ -2349,7 +2349,7 @@ bool IsOverrideNeededByInstantiatedTypeDueToPreservedScope (MethodDefinition met if (IgnoreScope (@base.DeclaringType.Scope)) return true; - if (IsVirtualNeededByTypeDueToPreservedScope (@base)) + if (IsMethodNeededByTypeDueToPreservedScope (@base)) return true; } @@ -3137,7 +3137,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type protected virtual IEnumerable GetRequiredMethodsForInstantiatedType (TypeDefinition type) { foreach (var method in type.Methods) { - if (IsOverrideNeededByInstantiatedTypeDueToPreservedScope (method)) + if (IsMethodNeededByInstantiatedTypeDueToPreservedScope (method)) yield return method; } } diff --git a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs new file mode 100644 index 000000000000..0903f70e657a --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs @@ -0,0 +1,23 @@ +// 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.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Mono.Linker.Tests.Cases.Libraries.Dependencies +{ + public interface ICopyLibraryInterface + { + void CopyLibraryInterfaceMethod (); + void CopyLibraryExplicitImplementationInterfaceMethod (); + } + + public interface ICopyLibraryStaticInterface + { + static abstract void CopyLibraryStaticInterfaceMethod (); + static abstract void CopyLibraryExplicitImplementationStaticInterfaceMethod (); + } +} diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index d86b9085a338..9b3e96b641fa 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -7,9 +7,12 @@ using System.Runtime.Serialization; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.Libraries.Dependencies; namespace Mono.Linker.Tests.Cases.Libraries { + [SetupCompileBefore ("copylibrary.dll", new[] { "Dependencies/CopyLibrary.cs" })] + [SetupLinkerAction ("copy", "copylibrary")] [SetupLinkerArgument ("-a", "test.exe", "library")] [SetupLinkerArgument ("--enable-opt", "ipconstprop")] [VerifyMetadataNames] @@ -180,10 +183,45 @@ public interface I [Kept] [KeptInterface (typeof (IEnumerator))] - public class UninstantiatedPublicClassWithInterface : IEnumerator + [KeptInterface (typeof (IPublicInterface))] + [KeptInterface (typeof (IPublicStaticInterface))] + [KeptInterface (typeof (IInternalInterface))] + [KeptInterface (typeof (IInternalStaticInterface))] + [KeptInterface (typeof (ICopyLibraryInterface))] + [KeptInterface (typeof (ICopyLibraryStaticInterface))] + public class UninstantiatedPublicClassWithInterface : + IPublicInterface, + IPublicStaticInterface, + IInternalInterface, + IInternalStaticInterface, + IEnumerator, + ICopyLibraryInterface, + ICopyLibraryStaticInterface { internal UninstantiatedPublicClassWithInterface () { } + [Kept] + public void PublicInterfaceMethod () { } + + [Kept] + void IPublicInterface.ExplicitImplementationPublicInterfaceMethod () { } + + [Kept] + public static void PublicStaticInterfaceMethod () { } + + [Kept] + static void IPublicStaticInterface.ExplicitImplementationPublicStaticInterfaceMethod () { } + + [Kept] + public void InternalInterfaceMethod () { } + + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + + [Kept] + public static void InternalStaticInterfaceMethod () { } + + static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { } + [Kept] bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); } @@ -192,6 +230,18 @@ internal UninstantiatedPublicClassWithInterface () { } [Kept] void IEnumerator.Reset () { } + + [Kept] + public void CopyLibraryInterfaceMethod () { } + + [Kept] + void ICopyLibraryInterface.CopyLibraryExplicitImplementationInterfaceMethod () { } + + [Kept] + public static void CopyLibraryStaticInterfaceMethod () { } + + [Kept] + static void ICopyLibraryStaticInterface.CopyLibraryExplicitImplementationStaticInterfaceMethod () { } } [Kept] @@ -204,6 +254,8 @@ internal UninstantiatedPublicClassWithImplicitlyImplementedInterface () { } [Kept] public void InternalInterfaceMethod () { } + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + [Kept] public string ToString (string format, IFormatProvider formatProvider) { @@ -212,17 +264,67 @@ public string ToString (string format, IFormatProvider formatProvider) } [Kept] + [KeptInterface (typeof (IEnumerator))] [KeptInterface (typeof (IPublicInterface))] + [KeptInterface (typeof (IPublicStaticInterface))] [KeptInterface (typeof (IInternalInterface))] - public class InstantiatedClassWithInterfaces : IPublicInterface, IInternalInterface + [KeptInterface (typeof (IInternalStaticInterface))] + [KeptInterface (typeof (ICopyLibraryInterface))] + [KeptInterface (typeof (ICopyLibraryStaticInterface))] + public class InstantiatedClassWithInterfaces : + IPublicInterface, + IPublicStaticInterface, + IInternalInterface, + IInternalStaticInterface, + IEnumerator, + ICopyLibraryInterface, + ICopyLibraryStaticInterface { [Kept] public InstantiatedClassWithInterfaces () { } [Kept] - void IPublicInterface.PublicInterfaceMethod () { } + public void PublicInterfaceMethod () { } + + [Kept] + void IPublicInterface.ExplicitImplementationPublicInterfaceMethod () { } + + [Kept] + public static void PublicStaticInterfaceMethod () { } + + [Kept] + static void IPublicStaticInterface.ExplicitImplementationPublicStaticInterfaceMethod () { } + + [Kept] + public void InternalInterfaceMethod () { } - void IInternalInterface.InternalInterfaceMethod () { } + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + + [Kept] + public static void InternalStaticInterfaceMethod () { } + + static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { } + + [Kept] + bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); } + + [Kept] + object IEnumerator.Current { [Kept] get { throw new PlatformNotSupportedException (); } } + + [Kept] + void IEnumerator.Reset () { } + + [Kept] + public void CopyLibraryInterfaceMethod () { } + + [Kept] + void ICopyLibraryInterface.CopyLibraryExplicitImplementationInterfaceMethod () { } + + [Kept] + public static void CopyLibraryStaticInterfaceMethod () { } + + [Kept] + static void ICopyLibraryStaticInterface.CopyLibraryExplicitImplementationStaticInterfaceMethod () { } } [Kept] @@ -239,12 +341,36 @@ public interface IPublicInterface { [Kept] void PublicInterfaceMethod (); + + [Kept] + void ExplicitImplementationPublicInterfaceMethod (); + } + + [Kept] + public interface IPublicStaticInterface + { + [Kept] + static abstract void PublicStaticInterfaceMethod (); + + [Kept] + static abstract void ExplicitImplementationPublicStaticInterfaceMethod (); } [Kept] internal interface IInternalInterface { void InternalInterfaceMethod (); + + void ExplicitImplementationInternalInterfaceMethod (); + } + + [Kept] + internal interface IInternalStaticInterface + { + [Kept] // TODO: This is probably wrong + static abstract void InternalStaticInterfaceMethod (); + + static abstract void ExplicitImplementationInternalStaticInterfaceMethod (); } [Kept] diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs index 8aeb92da83ed..991c1860e4a8 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseCompiler.cs @@ -234,7 +234,7 @@ protected virtual NPath CompileCSharpAssemblyWithDefaultCompiler (CompilerOption #if NETCOREAPP protected virtual NPath CompileCSharpAssemblyWithRoslyn (CompilerOptions options) { - var languageVersion = LanguageVersion.Default; + var languageVersion = LanguageVersion.Preview; var compilationOptions = new CSharpCompilationOptions ( outputKind: options.OutputPath.FileName.EndsWith (".exe") ? OutputKind.ConsoleApplication : OutputKind.DynamicallyLinkedLibrary, assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default From 4ea299166e41091ee458d1781da0a314672dd2e5 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 8 Apr 2022 11:36:59 -0700 Subject: [PATCH 13/13] More tests for interface behavior --- .../Inheritance.InterfacesTests.g.cs | 6 + .../Dependencies/CopyLibrary.cs | 17 ++ .../InterfaceVariants.cs | 242 ++++++++++++++++++ .../Libraries/Dependencies/CopyLibrary.cs | 6 - .../Libraries/RootLibrary.cs | 8 +- 5 files changed, 269 insertions(+), 10 deletions(-) create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/Dependencies/CopyLibrary.cs create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs diff --git a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.InterfacesTests.g.cs b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.InterfacesTests.g.cs index 7b604a38ad55..74ea1c72d468 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.InterfacesTests.g.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.InterfacesTests.g.cs @@ -21,5 +21,11 @@ public Task InterfaceOnUninstantiatedTypeRemoved () return RunTest (allowMissingWarnings: true); } + [Fact] + public Task InterfaceVariants () + { + return RunTest (allowMissingWarnings: true); + } + } } \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/Dependencies/CopyLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/Dependencies/CopyLibrary.cs new file mode 100644 index 000000000000..655198709b1c --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/Dependencies/CopyLibrary.cs @@ -0,0 +1,17 @@ +// 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. + +namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.Dependencies +{ + public interface ICopyLibraryInterface + { + void CopyLibraryInterfaceMethod (); + void CopyLibraryExplicitImplementationInterfaceMethod (); + } + + public interface ICopyLibraryStaticInterface + { + static abstract void CopyLibraryStaticInterfaceMethod (); + static abstract void CopyLibraryExplicitImplementationStaticInterfaceMethod (); + } +} diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs new file mode 100644 index 000000000000..f9d54d009cf1 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -0,0 +1,242 @@ +// 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 Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; +using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.Inheritance.Interfaces.Dependencies; + +namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces +{ + [SetupCompileBefore ("copylibrary.dll", new[] { "Dependencies/CopyLibrary.cs" })] + [SetupLinkerAction ("copy", "copylibrary")] + public class InterfaceVariants + { + public static void Main () + { + Type t = typeof (UninstantiatedPublicClassWithInterface); + t = typeof (UninstantiatedPublicClassWithImplicitlyImplementedInterface); + t = typeof (UninstantiatedPublicClassWithPrivateInterface); + + UninstantiatedPublicClassWithInterface.InternalStaticInterfaceMethodUsed (); + InstantiatedClassWithInterfaces.InternalStaticInterfaceMethodUsed (); + + // Use all public interfaces - they're marked as public only to denote them as "used" + typeof (IPublicInterface).RequiresPublicMethods (); + typeof (IPublicStaticInterface).RequiresPublicMethods (); + + var a = new InstantiatedClassWithInterfaces (); + } + + [Kept] + [KeptInterface (typeof (IEnumerator))] + [KeptInterface (typeof (IPublicInterface))] + [KeptInterface (typeof (IPublicStaticInterface))] + [KeptInterface (typeof (IInternalStaticInterfaceWithUsedMethod))] // https://github.com/dotnet/linker/issues/2733 + [KeptInterface (typeof (ICopyLibraryInterface))] + [KeptInterface (typeof (ICopyLibraryStaticInterface))] + public class UninstantiatedPublicClassWithInterface : + IPublicInterface, + IPublicStaticInterface, + IInternalInterface, + IInternalStaticInterface, + IInternalStaticInterfaceWithUsedMethod, + IEnumerator, + ICopyLibraryInterface, + ICopyLibraryStaticInterface + { + internal UninstantiatedPublicClassWithInterface () { } + + [Kept] + public void PublicInterfaceMethod () { } + + [Kept] + void IPublicInterface.ExplicitImplementationPublicInterfaceMethod () { } + + [Kept] + public static void PublicStaticInterfaceMethod () { } + + [Kept] + static void IPublicStaticInterface.ExplicitImplementationPublicStaticInterfaceMethod () { } + + public void InternalInterfaceMethod () { } + + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + + public static void InternalStaticInterfaceMethod () { } + + static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { } + + [Kept] + public static void InternalStaticInterfaceMethodUsed () { } + + [Kept] + [ExpectBodyModified] + bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); } + + [Kept] + object IEnumerator.Current { + [Kept] + [ExpectBodyModified] + get { throw new PlatformNotSupportedException (); } + } + + [Kept] + void IEnumerator.Reset () { } + + [Kept] + public void CopyLibraryInterfaceMethod () { } + + [Kept] + void ICopyLibraryInterface.CopyLibraryExplicitImplementationInterfaceMethod () { } + + [Kept] + public static void CopyLibraryStaticInterfaceMethod () { } + + [Kept] + static void ICopyLibraryStaticInterface.CopyLibraryExplicitImplementationStaticInterfaceMethod () { } + } + + [Kept] + [KeptInterface (typeof (IFormattable))] + public class UninstantiatedPublicClassWithImplicitlyImplementedInterface : IInternalInterface, IFormattable + { + internal UninstantiatedPublicClassWithImplicitlyImplementedInterface () { } + + public void InternalInterfaceMethod () { } + + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + + [Kept] + [ExpectBodyModified] + [ExpectLocalsModified] + public string ToString (string format, IFormatProvider formatProvider) + { + return "formatted string"; + } + } + + [Kept] + [KeptInterface (typeof (IEnumerator))] + [KeptInterface (typeof (IPublicInterface))] + [KeptInterface (typeof (IPublicStaticInterface))] + [KeptInterface (typeof (IInternalStaticInterfaceWithUsedMethod))] // https://github.com/dotnet/linker/issues/2733 + [KeptInterface (typeof (ICopyLibraryInterface))] + [KeptInterface (typeof (ICopyLibraryStaticInterface))] + public class InstantiatedClassWithInterfaces : + IPublicInterface, + IPublicStaticInterface, + IInternalInterface, + IInternalStaticInterface, + IInternalStaticInterfaceWithUsedMethod, + IEnumerator, + ICopyLibraryInterface, + ICopyLibraryStaticInterface + { + [Kept] + public InstantiatedClassWithInterfaces () { } + + [Kept] + public void PublicInterfaceMethod () { } + + [Kept] + void IPublicInterface.ExplicitImplementationPublicInterfaceMethod () { } + + [Kept] + public static void PublicStaticInterfaceMethod () { } + + [Kept] + static void IPublicStaticInterface.ExplicitImplementationPublicStaticInterfaceMethod () { } + + public void InternalInterfaceMethod () { } + + void IInternalInterface.ExplicitImplementationInternalInterfaceMethod () { } + + public static void InternalStaticInterfaceMethod () { } + + static void IInternalStaticInterface.ExplicitImplementationInternalStaticInterfaceMethod () { } + + [Kept] + public static void InternalStaticInterfaceMethodUsed () { } + + [Kept] + bool IEnumerator.MoveNext () { throw new PlatformNotSupportedException (); } + + [Kept] + object IEnumerator.Current { [Kept] get { throw new PlatformNotSupportedException (); } } + + [Kept] + void IEnumerator.Reset () { } + + [Kept] + public void CopyLibraryInterfaceMethod () { } + + [Kept] + void ICopyLibraryInterface.CopyLibraryExplicitImplementationInterfaceMethod () { } + + [Kept] + public static void CopyLibraryStaticInterfaceMethod () { } + + [Kept] + static void ICopyLibraryStaticInterface.CopyLibraryExplicitImplementationStaticInterfaceMethod () { } + } + + [Kept] + public class UninstantiatedPublicClassWithPrivateInterface : IPrivateInterface + { + internal UninstantiatedPublicClassWithPrivateInterface () { } + + void IPrivateInterface.PrivateInterfaceMethod () { } + } + + [Kept] + public interface IPublicInterface + { + [Kept] + void PublicInterfaceMethod (); + + [Kept] + void ExplicitImplementationPublicInterfaceMethod (); + } + + [Kept] + public interface IPublicStaticInterface + { + [Kept] + static abstract void PublicStaticInterfaceMethod (); + + [Kept] + static abstract void ExplicitImplementationPublicStaticInterfaceMethod (); + } + + internal interface IInternalInterface + { + void InternalInterfaceMethod (); + + void ExplicitImplementationInternalInterfaceMethod (); + } + + internal interface IInternalStaticInterface + { + static abstract void InternalStaticInterfaceMethod (); + + static abstract void ExplicitImplementationInternalStaticInterfaceMethod (); + } + + // The interface methods themselves are not used, but the implentation of these methods is + // https://github.com/dotnet/linker/issues/2733 + [Kept] + internal interface IInternalStaticInterfaceWithUsedMethod + { + [Kept] // https://github.com/dotnet/linker/issues/2733 + static abstract void InternalStaticInterfaceMethodUsed (); + } + + private interface IPrivateInterface + { + void PrivateInterfaceMethod (); + } + } +} diff --git a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs index 0903f70e657a..0f31e8277c37 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyLibrary.cs @@ -1,12 +1,6 @@ // 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.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - namespace Mono.Linker.Tests.Cases.Libraries.Dependencies { public interface ICopyLibraryInterface diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index 9b3e96b641fa..82f78a8c80bf 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -1,9 +1,9 @@ +// 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.CodeAnalysis; -using System.Reflection; -using System.Runtime.CompilerServices; using System.Runtime.Serialization; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; @@ -367,7 +367,7 @@ internal interface IInternalInterface [Kept] internal interface IInternalStaticInterface { - [Kept] // TODO: This is probably wrong + [Kept] // https://github.com/dotnet/linker/issues/2733 static abstract void InternalStaticInterfaceMethod (); static abstract void ExplicitImplementationInternalStaticInterfaceMethod ();