Don't keep members of pointer or byref element types

src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs

@@ -245,8 +245,8 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy
 				return false;
 			}
 
-			if (!_reflectionMarker.TryResolveTypeNameAndMark (resolvedAssembly, typeName, _diagnosticContext, out TypeDefinition? resolvedTypeDefinition)
-				|| resolvedTypeDefinition.IsTypeOf (WellKnownType.System_Array)) {
+			if (!_reflectionMarker.TryResolveTypeNameAndMark (resolvedAssembly, typeName, _diagnosticContext, out TypeReference? foundType)
+				|| foundType.IsTypeOf (WellKnownType.System_Array)) {
 				// It's not wrong to have a reference to non-existing type - the code may well expect to get an exception in this case
 				// Note that we did find the assembly, so it's not a ILLink config problem, it's either intentional, or wrong versions of assemblies
 				// but ILLink can't know that. In case a user tries to create an array using System.Activator we should simply ignore it, the user
@@ -255,7 +255,7 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy
 				return false;
 			}
 
-			resolvedType = new TypeProxy (resolvedTypeDefinition, _context);
+			resolvedType = new TypeProxy (foundType, _context);
 			return true;
 		}
 

src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs

@@ -58,39 +58,32 @@ internal void MarkTypeForDynamicallyAccessedMembers (in MessageOrigin origin, Ty
 
 		// Resolve a (potentially assembly qualified) type name based on the current context (taken from DiagnosticContext) and mark the type for reflection.
 		// This method will probe the current context assembly and if that fails CoreLib for the specified type. Emulates behavior of Type.GetType.
-		internal bool TryResolveTypeNameAndMark (string typeName, in DiagnosticContext diagnosticContext, bool needsAssemblyName, [NotNullWhen (true)] out TypeDefinition? type)
+		internal bool TryResolveTypeNameAndMark (string typeName, in DiagnosticContext diagnosticContext, bool needsAssemblyName, [NotNullWhen (true)] out TypeReference? type)
 		{
-			if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out TypeReference? typeRef, out var typeResolutionRecords, needsAssemblyName)
-				|| typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition foundType) {
+			if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out type, out var typeResolutionRecords, needsAssemblyName)) {
 				type = default;
 				return false;
 			}
 
-			MarkResolvedType (diagnosticContext, typeRef, foundType, typeResolutionRecords);
-
-			type = foundType;
+			MarkType (diagnosticContext, type, typeResolutionRecords);
 			return true;
 		}
 
 		// Resolve a type from the specified assembly and mark it for reflection.
-		internal bool TryResolveTypeNameAndMark (AssemblyDefinition assembly, string typeName, in DiagnosticContext diagnosticContext, [NotNullWhen (true)] out TypeDefinition? type)
+		internal bool TryResolveTypeNameAndMark (AssemblyDefinition assembly, string typeName, in DiagnosticContext diagnosticContext, [NotNullWhen (true)] out TypeReference? type)
 		{
-			if (!_context.TypeNameResolver.TryResolveTypeName (assembly, typeName, out TypeReference? typeRef, out var typeResolutionRecords)
-				|| typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition foundType) {
+			if (!_context.TypeNameResolver.TryResolveTypeName (assembly, typeName, out type, out var typeResolutionRecords)) {
 				type = default;
 				return false;
 			}
 
-			MarkResolvedType (diagnosticContext, typeRef, foundType, typeResolutionRecords);
-
-			type = foundType;
+			MarkType (diagnosticContext, type, typeResolutionRecords);
 			return true;
 		}
 
-		void MarkResolvedType (
+		void MarkType (
 			in DiagnosticContext diagnosticContext,
 			TypeReference typeReference,
-			TypeDefinition typeDefinition,
 			List<TypeNameResolver.TypeResolutionRecord> typeResolutionRecords)
 		{
 			if (_enabled) {
@@ -100,9 +93,9 @@ void MarkResolvedType (
 				// This is necessary because if the app's code contains the input string as literal (which is pretty much always the case)
 				// that string has to work at runtime, and if it relies on type forwarders we need to preserve those as well.
 				var origin = diagnosticContext.Origin;
-				_markStep.MarkTypeVisibleToReflection (typeReference, typeDefinition, new DependencyInfo (DependencyKind.AccessedViaReflection, origin.Provider), origin);
+				_markStep.MarkTypeVisibleToReflection (typeReference, new DependencyInfo (DependencyKind.AccessedViaReflection, origin.Provider), origin);
 				foreach (var typeResolutionRecord in typeResolutionRecords) {
-					_context.MarkingHelpers.MarkMatchingExportedType (typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, typeDefinition), origin);
+					_context.MarkingHelpers.MarkMatchingExportedType (typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, typeReference), origin);
 				}
 			}
 		}
@@ -115,7 +108,7 @@ internal void MarkType (in MessageOrigin origin, TypeReference typeRef, Dependen
 			if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type)
 				return;
 
-			_markStep.MarkTypeVisibleToReflection (type, type, new DependencyInfo (dependencyKind, origin.Provider), origin);
+			_markStep.MarkTypeVisibleToReflection (type, new DependencyInfo (dependencyKind, origin.Provider), origin);
 		}
 
 		internal void MarkMethod (in MessageOrigin origin, MethodReference methodRef, DependencyKind dependencyKind = DependencyKind.AccessedViaReflection)

src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs

@@ -26,7 +26,7 @@ public RequireDynamicallyAccessedMembersAction (
 
 		public partial bool TryResolveTypeNameAndMark (string typeName, bool needsAssemblyName, out TypeProxy type)
 		{
-			if (_reflectionMarker.TryResolveTypeNameAndMark (typeName, _diagnosticContext, needsAssemblyName, out TypeDefinition? foundType)) {
+			if (_reflectionMarker.TryResolveTypeNameAndMark (typeName, _diagnosticContext, needsAssemblyName, out TypeReference? foundType)) {
 				type = new (foundType, _resolver);
 				return true;
 			} else {

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

@@ -325,7 +325,7 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason, Mes
 					MarkEntireType (nested, new DependencyInfo (DependencyKind.NestedType, type), origin);
 			}
 
-			MarkTypeVisibleToReflection (type, type, reason, origin);
+			MarkTypeVisibleToReflection (type, reason, origin);
 			MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type), origin);
 			MarkTypeSpecialCustomAttributes (type, origin);
 
@@ -914,7 +914,7 @@ void MarkMembersVisibleToReflection (IEnumerable<IMetadataTokenProvider> members
 			foreach (var member in members) {
 				switch (member) {
 				case TypeDefinition type:
-					MarkTypeVisibleToReflection (type, type, reason, origin);
+					MarkTypeVisibleToReflection (type, reason, origin);
 					break;
 				case MethodDefinition method:
 					MarkMethodVisibleToReflection (method, reason, origin);
@@ -1795,18 +1795,17 @@ protected virtual void MarkSerializable (TypeDefinition type, MessageOrigin orig
 			MarkMethodsIf (type.Methods, HasOnSerializeOrDeserializeAttribute, new DependencyInfo (DependencyKind.SerializationMethodForType, type), origin);
 		}
 
-		protected internal virtual TypeDefinition? MarkTypeVisibleToReflection (TypeReference type, TypeDefinition definition, in DependencyInfo reason, in MessageOrigin origin)
+		protected internal virtual void MarkTypeVisibleToReflection (TypeReference type, in DependencyInfo reason, in MessageOrigin origin)
 		{
-			// If a type is visible to reflection, we need to stop doing optimization that could cause observable difference
-			// in reflection APIs. This includes APIs like MakeGenericType (where variant castability of the produced type
-			// could be incorrect) or IsAssignableFrom (where assignability of unconstructed types might change).
-			Annotations.MarkRelevantToVariantCasting (definition);
-
-			Annotations.MarkReflectionUsed (definition);
-
-			MarkImplicitlyUsedFields (definition, origin);
-
-			return MarkType (type, reason, origin);
+			TypeDefinition? definition = MarkType (type, reason, origin);
+			if (definition is not null) {
+				// If a type is visible to reflection, we need to stop doing optimization that could cause observable difference
+				// in reflection APIs. This includes APIs like MakeGenericType (where variant castability of the produced type
+				// could be incorrect) or IsAssignableFrom (where assignability of unconstructed types might change).
+				Annotations.MarkRelevantToVariantCasting (definition);
+				Annotations.MarkReflectionUsed (definition);
+				MarkImplicitlyUsedFields (definition, origin);
+			}
 		}
 
 		internal void MarkMethodVisibleToReflection (MethodReference method, in DependencyInfo reason, in MessageOrigin origin)
@@ -3640,9 +3639,7 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
 					origin = new MessageOrigin (origin, instruction.Offset);
 
 					if (token is TypeReference typeReference) {
-						// Error will be reported as part of MarkType
-						if (Context.TryResolve (typeReference) is TypeDefinition type)
-							MarkTypeVisibleToReflection (typeReference, type, reason, origin);
+						MarkTypeVisibleToReflection (typeReference, reason, origin);
 					} else if (token is MethodReference methodReference) {
 						MarkMethodVisibleToReflection (methodReference, reason, origin);
 					} else {

src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs

@@ -432,13 +432,18 @@ public static bool IsNamedType (this TypeReference typeReference) {
 			return true;
 		}
 
-		// Array types that are dynamically accessed should resolve to System.Array instead of its element type - which is what Cecil resolves to.
-		// Any data flow annotations placed on a type parameter which receives an array type apply to the array itself. None of the members in its
-		// element type should be marked.
+		/// <summary>
+		/// Resolves a TypeReference to a TypeDefinition if possible. Non-named types other than arrays (pointers, byrefs, function pointers) return null.
+		/// Array types that are dynamically accessed resolve to System.Array instead of its element type - which is what Cecil resolves to.
+		/// Any data flow annotations placed on a type parameter which receives an array type apply to the array itself. None of the members in its
+		/// element type should be marked.
+		/// </summary>
 		public static TypeDefinition? ResolveToTypeDefinition (this TypeReference typeReference, LinkContext context)
 			=> typeReference is ArrayType
 				? BCL.FindPredefinedType (WellKnownType.System_Array, context)
-				: context.TryResolve (typeReference);
+				: typeReference.IsNamedType ()
+					? context.TryResolve (typeReference)
+					: null;
 
 		public static bool IsByRefOrPointer (this TypeReference typeReference)
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs

@@ -60,6 +60,7 @@ public static void Main ()
 			TestInvalidTypeCombination ();
 			TestEscapedTypeName ();
 			AssemblyTypeResolutionBehavior.Test ();
+			InstantiatedGenericEquality.Test ();
 		}
 
 		[Kept]
@@ -213,15 +214,11 @@ public static void TestType ()
 		}
 
 		[Kept]
-#if !NATIVEAOT // https://github.com/dotnet/runtime/issues/106214
-		[KeptMember (".ctor()")]
-#endif
 		[KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
 		[RequiresUnreferencedCode (nameof (Pointer))]
 		public class Pointer { }
 
 		[Kept]
-		[UnexpectedWarning ("IL2026", nameof (Pointer), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/106214")]
 		public static void TestPointer ()
 		{
 			const string reflectionTypeKeptString = "Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+Pointer*";
@@ -230,15 +227,11 @@ public static void TestPointer ()
 		}
 
 		[Kept]
-#if !NATIVEAOT // https://github.com/dotnet/runtime/issues/106214
-		[KeptMember (".ctor()")]
-#endif
 		[KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
 		[RequiresUnreferencedCode (nameof (Reference))]
 		public class Reference { }
 
 		[Kept]
-		[UnexpectedWarning ("IL2026", nameof (Reference), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/106214")]
 		public static void TestReference ()
 		{
 			const string reflectionTypeKeptString = "Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+Reference&";
@@ -686,6 +679,32 @@ class PointerElementGenericArgumentType {}
 			class ByRefElementGenericArgumentType {}
 		}
 
+		[Kept]
+		class InstantiatedGenericEquality
+		{
+			[Kept]
+			class Generic<T> {
+				[Kept]
+				public void Method () { }
+			}
+
+			// Regression test for an issue where ILLink's representation of a generic instantiated type
+			// was using reference equality. The test uses a lambda to ensure that it goes through the
+			// interprocedural analysis code path that merges patterns and relies on a correct implementation
+			// of equality.
+			[Kept]
+			public static void Test ()
+			{
+				var type = Type.GetType("Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+InstantiatedGenericEquality+Generic`1[[System.Int32]]");
+
+				var lambda = () => {
+					type.GetMethod ("Method");
+				};
+
+				lambda ();
+			}
+		}
+
 		[Kept]
 		static void RequireConstructor (
 			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]