Skip to content

Commit

Permalink
Support field access on variables of type Enum (#105524)
Browse files Browse the repository at this point in the history
Follow-up to #105351

This adds support for the case when a variable is of type Enum and
there are no generics involved, so the following no longer produces
trim warnings:

```csharp
static void M(Enum v) {
    v.GetType().GetFields();
}
```

Fixes #105506
  • Loading branch information
sbomer authored Jul 26, 2024
1 parent c1fdef4 commit 7102706
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 11 deletions.
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

0 comments on commit 7102706

Please sign in to comment.