WIP : Master base sweeping simplified

linker/Linker.Steps/BaseMarkingUtils.cs

@@ -0,0 +1,107 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Mono.Cecil;
+
+namespace Mono.Linker.Steps {
+	public static class BaseMarkingUtils {
+		public static bool ShouldMarkTypeHierarchyForMethod (LinkContext context, MethodReference method, TypeDefinition visibilityScope)
+		{
+			if (!NeedToCheckTypeHierarchy (context, visibilityScope))
+				return false;
+
+			if (IsTypeHierarchyRequiredForMethod (context, method, visibilityScope))
+				return true;
+
+			return false;
+		}
+
+		public static bool ShouldMarkTypeHierarchyForField (LinkContext context, FieldReference field, TypeDefinition visibilityScope)
+		{
+			if (!NeedToCheckTypeHierarchy (context, visibilityScope))
+				return false;
+
+			if (IsTypeHierarchyRequiredForField (context, field, visibilityScope))
+				return true;
+
+			return false;
+		}
+
+		static bool NeedToCheckTypeHierarchy (LinkContext context, TypeDefinition visibilityScope)
+		{
+			// We do not currently change the base type of value types
+			if (!visibilityScope.IsClass)
+				return false;
+
+			var basesOfScope = context.Annotations.GetBaseHierarchy (visibilityScope);
+
+			// No need to do this for types derived from object.  It already has the lowest base class
+			if (basesOfScope == null || basesOfScope.Count == 0)
+				return false;
+
+			return true;
+		}
+
+		static bool IsTypeHierarchyRequiredForField (LinkContext context, FieldReference field, TypeDefinition visibilityScope)
+		{
+			var resolved = field.Resolve ();
+			if (resolved == null) {
+				// Play it safe if we fail to resolve
+				return true;
+			}
+
+			var basesOfScope = context.Annotations.GetBaseHierarchy (visibilityScope);
+			var fromBase = basesOfScope.FirstOrDefault (b => resolved.DeclaringType == b);
+			if (fromBase != null) {
+				if (!resolved.IsStatic)
+					return true;
+
+				if (resolved.IsPublic)
+					return false;
+
+				// protected
+				if (resolved.IsFamily)
+					return true;
+
+				// It must be internal.  Trust that if the compiler allowed it we can continue to access
+				if (!resolved.IsPrivate)
+					return false;
+
+				return false;
+			}
+
+			return false;
+		}
+
+		static bool IsTypeHierarchyRequiredForMethod (LinkContext context, MethodReference method, TypeDefinition visibilityScope)
+		{
+			var resolved = method.Resolve ();
+			if (resolved == null) {
+				// Play it safe if we fail to resolve
+				return true;
+			}
+
+			var basesOfScope = context.Annotations.GetBaseHierarchy (visibilityScope);
+			var fromBase = basesOfScope.FirstOrDefault (b => resolved.DeclaringType == b);
+			if (fromBase != null) {
+				if (!resolved.IsStatic)
+					return true;
+
+				if (resolved.IsPublic)
+					return false;
+
+				// protected
+				if (resolved.IsFamily)
+					return true;
+
+				// It must be internal.  Trust that if the compiler allowed it we can continue to access
+				if (!resolved.IsPrivate)
+					return false;
+
+				return false;
+			}
+
+			return false;
+		}
+	}
+}

linker/Linker.Steps/MarkStep.cs

@@ -46,6 +46,7 @@ public partial class MarkStep : IStep {
 		protected Queue<AttributeProviderPair> _assemblyLevelAttributes;
 		protected Queue<AttributeProviderPair> _lateMarkedAttributes;
 		protected List<TypeDefinition> _typesWithInterfaces;
+		protected HashSet<TypeDefinition> _baseTypeHierarchyMarked;
 
 		public AnnotationStore Annotations {
 			get { return _context.Annotations; }
@@ -64,6 +65,7 @@ public MarkStep ()
 			_assemblyLevelAttributes = new Queue<AttributeProviderPair> ();
 			_lateMarkedAttributes = new Queue<AttributeProviderPair> ();
 			_typesWithInterfaces = new List<TypeDefinition> ();
+			_baseTypeHierarchyMarked = new HashSet<TypeDefinition>();
 		}
 
 		public virtual void Process (LinkContext context)
@@ -262,9 +264,16 @@ void ProcessOverride (MethodDefinition method, MethodDefinition @base)
 			if (Annotations.IsMarked (method))
 				return;
 
+			var isInstantiated = Annotations.IsInstantiated (method.DeclaringType);
+			var isBaseRequired = Annotations.IsBaseRequired (method.DeclaringType);
+
+			// We don't need to mark overrides interfaces until it is possible that the type could be instantiated
+			// if the type is not instantiated, interfaces will be removed so there is no need to mark interface methods
+			if (!isInstantiated && @base.DeclaringType.IsInterface)
+				return;
+
 			// We don't need to mark overrides until it is possible that the type could be instantiated
-			// Note : The base type is interface check should be removed once we have base type sweeping
-			if (!Annotations.IsInstantiated (method.DeclaringType) && @base.DeclaringType.IsInterface)
+			if (!isInstantiated && !isBaseRequired)
 				return;
 
 			MarkMethod (method);
@@ -870,6 +879,7 @@ protected void MarkField (FieldReference reference)
 			MarkCustomAttributes (field);
 			MarkMarshalSpec (field);
 			DoAdditionalFieldProcessing (field);
+			MarkTypeHierarchyRequirementsCausedByField (field, field.DeclaringType);
 
 			var parent = reference.DeclaringType.Resolve ();
 			if (!Annotations.HasPreservedStaticCtor (parent))
@@ -937,7 +947,6 @@ protected virtual TypeDefinition MarkType (TypeReference reference)
 			Tracer.Push (type);
 
 			MarkScope (type.Scope);
-			MarkType (type.BaseType);
 			MarkType (type.DeclaringType);
 			MarkCustomAttributes (type);
 			MarkSecurityDeclarations (type);
@@ -986,6 +995,8 @@ protected virtual TypeDefinition MarkType (TypeReference reference)
 				MarkRequirementsForInstantiatedTypes (type);
 			}
 
+			MarkBaseTypeRequiredIfNecessary (type);
+
 			if (type.HasInterfaces)
 				_typesWithInterfaces.Add (type);
 
@@ -1710,9 +1721,17 @@ protected virtual void ProcessMethod (MethodDefinition method)
 			MarkSecurityDeclarations (method);
 
 			MarkGenericParameterProvider (method);
+			MarkTypeHierarchyRequirementsCausedByMethodVisibility (method, method.DeclaringType);
 
-			if (ShouldMarkAsInstancePossible (method))
+			if (ShouldMarkAsInstancePossible (method)) {
 				MarkRequirementsForInstantiatedTypes (method.DeclaringType);
+
+				// If it's an extern ctor such as EventHandler's ctor, we won't have any IL to
+				// cause a ctor to be marked in the base type.   We know the base type needs to retain it's base,
+				// and that goes for all base types in the hierarchy 
+				if (!method.HasBody)
+					MarkBaseHierarchyAsRequired (method.DeclaringType);
+			}
 
 			if (IsPropertyMethod (method))
 				MarkProperty (GetProperty (method));
@@ -1744,6 +1763,7 @@ protected virtual void ProcessMethod (MethodDefinition method)
 			MarkBaseMethods (method);
 
 			MarkType (method.ReturnType);
+			MarkBaseHierarchyAsRequired (method.ReturnType);
 			MarkCustomAttributes (method.MethodReturnType);
 			MarkMarshalSpec (method.MethodReturnType);
 
@@ -1767,6 +1787,36 @@ protected virtual void DoAdditionalMethodProcessing (MethodDefinition method)
 		{
 		}
 
+		void MarkBaseHierarchyAsRequired (TypeReference type)
+		{
+			var resolved = type.Resolve ();
+			if (resolved == null)
+				return;
+
+			MarkBaseHierarchyAsRequired (resolved);
+		}
+
+		void MarkBaseHierarchyAsRequired (TypeDefinition type)
+		{
+			_baseTypeHierarchyMarked.Add (type);
+			Annotations.MarkBaseRequired (type);
+			if (type.BaseType != null)
+				MarkType (type.BaseType);
+
+			// We could resolve the base and recursively call ourselves but already have a list
+			// of the bases built up so let's just use that.
+			var bases = Annotations.GetBaseHierarchy (type);
+			if (bases == null)
+				return;
+
+			foreach (var @base in bases) {
+				_baseTypeHierarchyMarked.Add (type);
+				Annotations.MarkBaseRequired (@base);
+				if (@base.BaseType != null)
+					MarkType (@base.BaseType);
+			}
+		}
+
 		protected virtual bool ShouldMarkAsInstancePossible (MethodDefinition method)
 		{
 			// We don't need to mark it multiple times
@@ -1788,7 +1838,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type
 				return;
 
 			Annotations.MarkInstantiated (type);
-
+			MarkBaseHierarchyAsRequired (type);
 			MarkInterfaceImplementations (type);
 			MarkMethodsIf (type.Methods, IsVirtualAndHasPreservedParent);
 			DoAdditionalInstantiatedTypeProcessing (type);
@@ -1849,14 +1899,127 @@ void MarkNewCodeDependencies (MethodDefinition method)
 				_context.MarkedKnownMembers.NotSupportedExceptionCtorString = nseCtor;
 				break;
 			}
-		}		
+		}
+
+		bool MarkBaseTypeRequiredIfNecessary (TypeDefinition type)
+		{
+			if (!type.IsClass || type.BaseType == null)
+				return false;
+
+			var bases = Annotations.GetBaseHierarchy(type);
+
+			// Don't mess with attributes types.  Seems like asking for trouble since reflection is often used with attributes
+			// which means we likely cannot detect how the attributes are being used
+			if (bases.Any (t => t.FullName == "System.Attribute")) {
+				MarkBaseHierarchyAsRequired (type);
+				return true;
+			}
+
+			//
+			//Figuring out the visibility rules when nested classes are involved gets very complicated
+			// and is likely not worth the effort to implement.  Instead of trying to implement that, this method is here to
+			// mark the base type required in any situations we are not going to try and detect
+			//
+
+			foreach (var @base in bases) {
+				if (@base.HasNestedTypes) {
+					MarkBaseHierarchyAsRequired (type);
+					return true;
+				}
+			}
+
+			if (type.DeclaringType != null) {
+				if (MarkBaseTypeRequiredIfNecessary (type.DeclaringType)) {
+					MarkBaseHierarchyAsRequired (type);
+					return true;
+				}
+			}
+
+			return false;
+		}
+
+		void MarkBaseRequirements (MethodBody body)
+		{
+			var type = body.Method.DeclaringType;
+
+			// We do not currently change the base type of value types
+			if (type.IsValueType)
+				return;
+
+			foreach (var instruction in body.Instructions) {
+				if (instruction.Operand == null)
+					continue;
+
+				if (instruction.Operand is FieldReference fieldReference) {
+					MarkBaseRequirementsFromBody (fieldReference, body);
+				} else if (instruction.Operand is MethodReference methodReference) {
+					MarkBaseRequirementsFromBody(methodReference, body);
+				} else if (instruction.OpCode.Code == Code.Castclass || instruction.OpCode.Code == Code.Isinst) {
+					var typeReference = (instruction.Operand as TypeReference);
+					if (typeReference == null)
+						continue;
+					MarkBaseHierarchyAsRequired(typeReference);
+				}
+			}
+		}
+
+		void MarkBaseRequirementsFromBody (FieldReference field, MethodBody body)
+		{
+			MarkTypeHierarchyRequirementsCausedByField (field, body.Method.DeclaringType);
+		}
+
+		void MarkBaseRequirementsFromBody (MethodReference method, MethodBody body)
+		{
+			var visibilityScope = body.Method.DeclaringType;
+			MarkTypeHierarchyRequirementsCausedByMethodConstraints (method);
+			MarkTypeHierarchyRequirementsCausedByMethodVisibility (method, visibilityScope);
+		}
+
+		void MarkTypeHierarchyRequirementsCausedByField (FieldReference field, TypeDefinition visibilityScope)
+		{
+			if (!_baseTypeHierarchyMarked.Contains (visibilityScope) && BaseMarkingUtils.ShouldMarkTypeHierarchyForField (_context, field, visibilityScope))
+				MarkBaseHierarchyAsRequired (visibilityScope);
+		}
+
+		void MarkTypeHierarchyRequirementsCausedByMethodConstraints (MethodReference method)
+		{
+			var resolvedMethod = method.Resolve ();
+			if (resolvedMethod == null)
+				return;
+
+			// This is a little overly simplistic, we could be more precise, but at the moment it's not worth the added complexity.
+			// If the method has any constraints, mark each generic argument as needing it's base.
+			// Technically, we only need to do this for the generic arguments that have constraints, but this is such niche edge case
+			// that it's not worth the added complexity
+			if (resolvedMethod.GenericParameters.Any (param => param.HasConstraints)) {
+				if (method is IGenericInstance genericInstanceMethod) {
+					foreach (var genericArgumentType in genericInstanceMethod.GenericArguments) {
+						var resolvedGenericArgumentType = genericArgumentType.Resolve ();
+						if (resolvedGenericArgumentType == null || _baseTypeHierarchyMarked.Contains (resolvedGenericArgumentType))
+							continue;
+
+						MarkBaseHierarchyAsRequired (resolvedGenericArgumentType);
+					}
+				}
+			}
+		}
+
+		void MarkTypeHierarchyRequirementsCausedByMethodVisibility (MethodReference method, TypeDefinition visibilityScope)
+		{
+			if (!_baseTypeHierarchyMarked.Contains (visibilityScope) && BaseMarkingUtils.ShouldMarkTypeHierarchyForMethod (_context, method, visibilityScope)) {
+				MarkBaseHierarchyAsRequired (visibilityScope);
+			}
+		}
 
 		void MarkBaseMethods (MethodDefinition method)
 		{
 			var base_methods = Annotations.GetBaseMethods (method);
 			if (base_methods == null)
 				return;
 
+			// Once an override method is marked for any reason we can no longer change the base class
+			MarkBaseHierarchyAsRequired (method.DeclaringType);
+
 			foreach (MethodDefinition base_method in base_methods) {
 				if (base_method.DeclaringType.IsInterface && !method.DeclaringType.IsInterface)
 					continue;
@@ -1982,8 +2145,10 @@ void MarkMethodIfNotNull (MethodReference method)
 
 		protected virtual void MarkMethodBody (MethodBody body)
 		{
-			foreach (VariableDefinition var in body.Variables)
+			foreach (VariableDefinition var in body.Variables) {
 				MarkType (var.VariableType);
+				MarkBaseHierarchyAsRequired (var.VariableType);
+			}
 
 			foreach (ExceptionHandler eh in body.ExceptionHandlers)
 				if (eh.HandlerType == ExceptionHandlerType.Catch)
@@ -1992,6 +2157,7 @@ protected virtual void MarkMethodBody (MethodBody body)
 			foreach (Instruction instruction in body.Instructions)
 				MarkInstruction (instruction);
 
+			MarkBaseRequirements (body);
 			MarkThingsUsedViaReflection (body);
 
 			PostMarkMethodBody (body);