Skip to content

Commit 3114f8e

Browse files
Copilotjkotas
Copilot
and
jkotas
authored
Fix Type.IsVisible and Type.ContainsGenericParameters for function pointers (dotnet#122224)
- [x] Understand the issue: `Type.IsVisible` and `Type.ContainsGenericParameters` don't handle function pointers correctly - [x] Modify `ContainsGenericParameters` in Type.Helpers.cs to check function pointer return and parameter types - [x] Modify `IsVisible` in Type.Helpers.cs to check function pointer return and parameter types - [x] Add tests for function pointer visibility with private types - [x] Add tests for function pointer ContainsGenericParameters with generic types - [x] Build and test the changes - [x] Run code review and address feedback - [x] Run CodeQL checker <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Abstract implementations of Type.IsVisible and Type.ContainsGenericParameters do not handle function pointers</issue_title> > <issue_description>### Description > > Abstract implementations of Type.IsVisible and Type.ContainsGenericParameters do not handle function pointers > > ### Reproduction Steps > > ``` > unsafe > { > Console.WriteLine(typeof(delegate* <string>).IsVisible); > Console.WriteLine(typeof(delegate* <PrivateType>).IsVisible); > } > > > class PrivateType { } > ``` > > ### Expected behavior > > `true false` with `dotnet run` (correct). > > > ### Actual behavior > > `true true` with PublishAot=true > > ### Regression? > > _No response_ > > ### Known Workarounds > > _No response_ > > ### Configuration > > _No response_ > > ### Other information > > _No response_</issue_description> > > <agent_instructions>Handle function pointers in these two methods in Type.Helpers.cs. Make sure to add tests</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> - Fixes dotnet#122223 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 86dfeb9 commit 3114f8e

3 files changed

Lines changed: 145 additions & 2 deletions

File tree

‎src/libraries/Common/tests/System/FunctionPointerTests.cs‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,32 @@ public static unsafe void GenericFunctionPointer()
189189
Assert.True(fcnPtr2.ContainsGenericParameters);
190190
}
191191

192+
[Fact]
193+
public static unsafe void FunctionPointerVisibility()
194+
{
195+
Type t = typeof(FunctionPointerHolder).Project();
196+
197+
// A function pointer with public return type should be visible
198+
Type fnPtrPublicReturn = t.GetField(nameof(FunctionPointerHolder.Field_Int), Bindings)!.FieldType;
199+
Assert.True(fnPtrPublicReturn.IsFunctionPointer);
200+
Assert.True(fnPtrPublicReturn.IsVisible);
201+
202+
// A function pointer with public parameter types should be visible
203+
Type fnPtrPublicParams = t.GetField(nameof(FunctionPointerHolder.Field_PublicParam), Bindings)!.FieldType;
204+
Assert.True(fnPtrPublicParams.IsFunctionPointer);
205+
Assert.True(fnPtrPublicParams.IsVisible);
206+
207+
// A function pointer with private return type should not be visible
208+
Type fnPtrPrivateReturn = t.GetField("Field_PrivateReturn", Bindings)!.FieldType;
209+
Assert.True(fnPtrPrivateReturn.IsFunctionPointer);
210+
Assert.False(fnPtrPrivateReturn.IsVisible);
211+
212+
// A function pointer with private parameter type should not be visible
213+
Type fnPtrPrivateParam = t.GetField("Field_PrivateParam", Bindings)!.FieldType;
214+
Assert.True(fnPtrPrivateParam.IsFunctionPointer);
215+
Assert.False(fnPtrPrivateParam.IsVisible);
216+
}
217+
192218
[Theory]
193219
[InlineData(nameof(FunctionPointerHolder.MethodReturnValue1),
194220
"MethodReturnValue1()",
@@ -270,7 +296,9 @@ private static void VerifyFieldOrProperty(Type fnPtrType)
270296

271297
private unsafe class FunctionPointerHolder
272298
{
273-
#pragma warning disable 0649
299+
// CS0649: Field is never assigned to (fields are used via reflection)
300+
// CS0169: Field is never used (private fields are accessed via reflection)
301+
#pragma warning disable 0649, 0169
274302
public delegate*<void> ToString_1;
275303
public delegate*unmanaged<void> ToString_2;
276304
public delegate*<int> ToString_3;
@@ -283,7 +311,12 @@ private unsafe class FunctionPointerHolder
283311

284312
public delegate* managed<int> Field_Int;
285313
public delegate* managed<MyClass> Field_MyClass;
286-
#pragma warning restore 0649
314+
315+
// Fields for visibility tests
316+
public delegate* managed<int, void> Field_PublicParam;
317+
private delegate* managed<PrivateClass> Field_PrivateReturn;
318+
private delegate* managed<PrivateClass, void> Field_PrivateParam;
319+
#pragma warning restore 0649, 0169
287320

288321
public delegate* managed<int> Prop_Int { get; }
289322
public delegate* managed<MyClass> Prop_MyClass { get; }
@@ -299,6 +332,7 @@ private unsafe class FunctionPointerHolder
299332

300333
public class MyClass { }
301334
public struct MyStruct { }
335+
private class PrivateClass { }
302336
}
303337
}
304338
}

‎src/libraries/System.Private.CoreLib/src/System/Type.Helpers.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,20 @@ public virtual bool ContainsGenericParameters
4949
if (IsGenericParameter)
5050
return true;
5151

52+
if (IsFunctionPointer)
53+
{
54+
if (GetFunctionPointerReturnType().ContainsGenericParameters)
55+
return true;
56+
57+
foreach (Type parameterType in GetFunctionPointerParameterTypes())
58+
{
59+
if (parameterType.ContainsGenericParameters)
60+
return true;
61+
}
62+
63+
return false;
64+
}
65+
5266
if (!IsGenericType)
5367
return false;
5468

@@ -88,6 +102,20 @@ public bool IsVisible
88102
if (HasElementType)
89103
return GetElementType()!.IsVisible;
90104

105+
if (IsFunctionPointer)
106+
{
107+
if (!GetFunctionPointerReturnType().IsVisible)
108+
return false;
109+
110+
foreach (Type parameterType in GetFunctionPointerParameterTypes())
111+
{
112+
if (!parameterType.IsVisible)
113+
return false;
114+
}
115+
116+
return true;
117+
}
118+
91119
Type type = this;
92120
while (type.IsNested)
93121
{

‎src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,6 +1509,87 @@ public enum NestedEnum
15091509
C
15101510
}
15111511
}
1512+
1513+
/// <summary>
1514+
/// A mock function pointer type used to test the abstract ContainsGenericParameters implementation in Type.Helpers.cs.
1515+
/// This inherits from Type directly (not MockType) so that it uses the base ContainsGenericParameters implementation.
1516+
/// </summary>
1517+
private sealed class MockFunctionPointerType_ContainsGenericParameters : Type
1518+
{
1519+
private readonly Type _returnType;
1520+
private readonly Type[] _parameterTypes;
1521+
private static readonly Exception s_unexpected = new Exception("Did not expect to be called.");
1522+
1523+
public MockFunctionPointerType_ContainsGenericParameters(Type returnType, Type[] parameterTypes)
1524+
{
1525+
_returnType = returnType;
1526+
_parameterTypes = parameterTypes ?? Type.EmptyTypes;
1527+
}
1528+
1529+
// Required for ContainsGenericParameters to work correctly
1530+
protected override bool HasElementTypeImpl() => false;
1531+
public override bool IsGenericParameter => false;
1532+
public override bool IsFunctionPointer => true;
1533+
public override Type GetFunctionPointerReturnType() => _returnType;
1534+
public override Type[] GetFunctionPointerParameterTypes() => _parameterTypes;
1535+
1536+
// Required abstract members
1537+
public override Assembly Assembly => throw s_unexpected;
1538+
public override string AssemblyQualifiedName => throw s_unexpected;
1539+
public override Type BaseType => throw s_unexpected;
1540+
public override string FullName => throw s_unexpected;
1541+
public override Guid GUID => throw s_unexpected;
1542+
public override Module Module => throw s_unexpected;
1543+
public override string Namespace => throw s_unexpected;
1544+
public override Type UnderlyingSystemType => throw s_unexpected;
1545+
public override string Name => throw s_unexpected;
1546+
public override ConstructorInfo[] GetConstructors(BindingFlags bindingAttr) => throw s_unexpected;
1547+
public override object[] GetCustomAttributes(bool inherit) => throw s_unexpected;
1548+
public override object[] GetCustomAttributes(Type attributeType, bool inherit) => throw s_unexpected;
1549+
public override Type GetElementType() => throw s_unexpected;
1550+
public override EventInfo GetEvent(string name, BindingFlags bindingAttr) => throw s_unexpected;
1551+
public override EventInfo[] GetEvents(BindingFlags bindingAttr) => throw s_unexpected;
1552+
public override FieldInfo GetField(string name, BindingFlags bindingAttr) => throw s_unexpected;
1553+
public override FieldInfo[] GetFields(BindingFlags bindingAttr) => throw s_unexpected;
1554+
public override Type GetInterface(string name, bool ignoreCase) => throw s_unexpected;
1555+
public override Type[] GetInterfaces() => throw s_unexpected;
1556+
public override MemberInfo[] GetMembers(BindingFlags bindingAttr) => throw s_unexpected;
1557+
public override MethodInfo[] GetMethods(BindingFlags bindingAttr) => throw s_unexpected;
1558+
public override Type GetNestedType(string name, BindingFlags bindingAttr) => throw s_unexpected;
1559+
public override Type[] GetNestedTypes(BindingFlags bindingAttr) => throw s_unexpected;
1560+
public override PropertyInfo[] GetProperties(BindingFlags bindingAttr) => throw s_unexpected;
1561+
public override object InvokeMember(string name, BindingFlags invokeAttr, Binder binder, object target, object[] args, ParameterModifier[] modifiers, System.Globalization.CultureInfo culture, string[] namedParameters) => throw s_unexpected;
1562+
public override bool IsDefined(Type attributeType, bool inherit) => throw s_unexpected;
1563+
protected override TypeAttributes GetAttributeFlagsImpl() => throw s_unexpected;
1564+
protected override ConstructorInfo GetConstructorImpl(BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers) => throw s_unexpected;
1565+
protected override MethodInfo GetMethodImpl(string name, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers) => throw s_unexpected;
1566+
protected override PropertyInfo GetPropertyImpl(string name, BindingFlags bindingAttr, Binder binder, Type returnType, Type[] types, ParameterModifier[] modifiers) => throw s_unexpected;
1567+
protected override bool IsArrayImpl() => throw s_unexpected;
1568+
protected override bool IsByRefImpl() => throw s_unexpected;
1569+
protected override bool IsCOMObjectImpl() => throw s_unexpected;
1570+
protected override bool IsPointerImpl() => throw s_unexpected;
1571+
protected override bool IsPrimitiveImpl() => throw s_unexpected;
1572+
}
1573+
1574+
[Fact]
1575+
public static void FunctionPointerContainsGenericParameters()
1576+
{
1577+
// Function pointer with non-generic return type and no parameters should not contain generic parameters
1578+
var fnPtrNonGeneric = new MockFunctionPointerType_ContainsGenericParameters(typeof(string), Type.EmptyTypes);
1579+
Assert.False(fnPtrNonGeneric.ContainsGenericParameters);
1580+
1581+
// Function pointer with generic return type should contain generic parameters
1582+
var fnPtrGenericReturn = new MockFunctionPointerType_ContainsGenericParameters(typeof(List<>), Type.EmptyTypes);
1583+
Assert.True(fnPtrGenericReturn.ContainsGenericParameters);
1584+
1585+
// Function pointer with non-generic return type but generic parameter type should contain generic parameters
1586+
var fnPtrGenericParam = new MockFunctionPointerType_ContainsGenericParameters(typeof(string), new Type[] { typeof(List<>) });
1587+
Assert.True(fnPtrGenericParam.ContainsGenericParameters);
1588+
1589+
// Function pointer with non-generic return type and non-generic parameter type should not contain generic parameters
1590+
var fnPtrAllNonGeneric = new MockFunctionPointerType_ContainsGenericParameters(typeof(string), new Type[] { typeof(int) });
1591+
Assert.False(fnPtrAllNonGeneric.ContainsGenericParameters);
1592+
}
15121593
}
15131594

15141595
public class NonGenericClass { }

0 commit comments

Comments
 (0)