Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support field access on variables of type Enum #105524

Merged
merged 1 commit into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,10 @@ private partial bool TryHandleIntrinsic (
// currently it won't do.

TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type;
if (staticType?.IsByRef == true)
{
staticType = ((ByRefType)staticType).ParameterType;
}
if (staticType is null || (!staticType.IsDefType && !staticType.IsArray))
{
DynamicallyAccessedMemberTypes annotation = default;
Expand Down Expand Up @@ -432,6 +436,10 @@ private partial bool TryHandleIntrinsic (
// we will also make it just work, even if the annotation doesn't match the usage.
AddReturnValue(new SystemTypeValue(staticType));
}
else if (staticType.IsTypeOf("System", "Enum"))
{
AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.PublicFields));
}
else
{
Debug.Assert(staticType is MetadataType || staticType.IsArray);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ private partial bool TryHandleIntrinsic (
// where a parameter is annotated and if something in the method sets a specific known type to it
// we will also make it just work, even if the annotation doesn't match the usage.
AddReturnValue (new SystemTypeValue (new (staticType)));
} else if (staticType.IsTypeOf ("System", "Enum")) {
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.PublicFields));
} else {
var annotation = FlowAnnotations.GetTypeAnnotation (staticType);
AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj, annotation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ private partial bool TryHandleIntrinsic (
// currently it won't do.

TypeReference? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type;
if (staticType?.IsByReference == true)
staticType = ((ByReferenceType) staticType).ElementType;
TypeDefinition? staticTypeDef = staticType?.ResolveToTypeDefinition (_context);
if (staticType is null || staticTypeDef is null) {
DynamicallyAccessedMemberTypes annotation = default;
Expand Down Expand Up @@ -153,6 +155,8 @@ private partial bool TryHandleIntrinsic (
// where a parameter is annotated and if something in the method sets a specific known type to it
// we will also make it just work, even if the annotation doesn't match the usage.
AddReturnValue (new SystemTypeValue (staticType));
} else if (staticTypeDef.IsTypeOf ("System", "Enum")) {
AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj, DynamicallyAccessedMemberTypes.PublicFields));
} else {
// Make sure the type is marked (this will mark it as used via reflection, which is sort of true)
// This should already be true for most cases (method params, fields, ...), but just in case
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public static void Test ()
class EnumTypeSatisfiesPublicFields
{
[Kept]
[ExpectedWarning ("IL2072")]
static void ParameterType (Enum instance)
{
instance.GetType ().RequiresPublicFields ();
Expand All @@ -113,7 +112,6 @@ class FieldType
public FieldType (Enum instance) => field = instance;

[Kept]
[ExpectedWarning ("IL2072")]
public void Test ()
{
field.GetType ().RequiresPublicFields ();
Expand All @@ -134,18 +132,36 @@ static Enum ReturnType ()
}

[Kept]
[ExpectedWarning ("IL2072")]
static void TestReturnType ()
{
ReturnType ().GetType ().RequiresPublicFields ();
}

[Kept]
static void OutParameter (out Enum value)
{
value = EnumType.Value;
}

[Kept]
// Analyzer doesn't assign a value to the out parameter after calling the OutParameter method,
// so when it looks up the value of the local 'value', it returns an empty value, and the
// GetType intrinsic handling can't see that the out param satisfies the public fields requirement.
// Similar for the other cases below.
[ExpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101734")]
static void TestOutParameter ()
{
OutParameter (out var value);
value.GetType ().RequiresPublicFields ();
}

[Kept]
public static void Test ()
{
ParameterType (EnumType.Value);
new FieldType (EnumType.Value).Test ();
TestReturnType ();
TestOutParameter ();
}
}

Expand Down Expand Up @@ -173,10 +189,10 @@ public static void TestAccessFromType ()

// Note: this doesn't warn for ILLink as a consequence of https://github.com/dotnet/runtime/issues/105345.
// ILLink sees the field type as a generic parameter, whereas the other tools see it as System.Enum.
// The special handling that treats Enum as satisfying PublicFields only applies to generic parameter constraints,
// so ILLink doesn't warn here. Once this 105345 is fixed, ILLink should match the warning behavior of ILC
// here and in the similar cases below.
[ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")]
// The other tools don't warn because of the built-in handling for variables of type System.Enum,
// and ILLink doesn't warn because this goes through the case that handles generic parameters constrained to be Enum.
// When https://github.com/dotnet/runtime/issues/105345 is fixed this should go through the built-in handling for
// System.Enum, like it does for ILC and the analyzer.
static void TestAccessTypeGenericParameterAsField ()
{
TypeGenericParameterAsField<Enum>.field.GetType ().RequiresPublicFields ();
Expand Down Expand Up @@ -214,7 +230,7 @@ public static void TestAccessFromType ()
}
}

[ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")]
// Note: this happens to work for ILLink due to https://github.com/dotnet/runtime/issues/105345.
static void TestAccessTypeGenericParameterAsReturnType ()
{
TypeGenericParameterAsReturnType<Enum>.Method ().GetType ().RequiresPublicFields ();
Expand All @@ -230,15 +246,15 @@ public static void Method (out T instance)
}

[Kept]
[ExpectedWarning ("IL2072")]
[ExpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101734")]
public static void TestAccessFromType ()
{
Method (out var instance);
instance.GetType ().RequiresPublicFields ();
}
}

[ExpectedWarning ("IL2072")]
[ExpectedWarning ("IL2072", Tool.Analyzer, "https://github.com/dotnet/runtime/issues/101734")]
static void TestAccessTypeGenericParameterAsOutParam ()
{
TypeGenericParameterAsOutParam<Enum>.Method (out var instance);
Expand All @@ -259,7 +275,7 @@ static T MethodGenericParameterAsReturnType<T> () where T : Enum
}

[Kept]
[ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")]
// Note: this happens to work for ILLink due to https://github.com/dotnet/runtime/issues/105345.
static void TestMethodGenericParameterAsReturnType ()
{
MethodGenericParameterAsReturnType<Enum> ().GetType ().RequiresPublicFields ();
Expand Down
Loading