Enable more ILLinker skipped tests on native AOT #110353
Conversation
Progress towards dotnet#82447.
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
| @@ -1188,7 +1188,7 @@ class FieldTypeAlsoImplementsInterface : IAnnotatedInterface | |||
| public NestedFieldType _nestedField; | |||
|
|
|||
| [Kept] | |||
| public class NestedFieldType | |||
| public struct NestedFieldType | |||
There was a problem hiding this comment.
Native AOT will still try to optimize out the MethodTable of a type even if it's used as the type of a reflection-visible field so this wasn't kept. It is not able to optimize it out if the type of the field is a struct since reflection APIs (FieldInfo.GetValue) will create a default-initialized boxed version when called.
(The test infrastructure is looking for MethodTables, not for type metadata. We would have the metadata only.)
|
@sbomer could you please have a look when you get a chance? |
| @@ -1557,17 +1557,17 @@ class UsedByDerived | |||
| { | |||
| class AnnotatedBase | |||
| { | |||
| [Kept] | |||
| [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] | |||
| [Kept (By = Tool.Trimmer /* The object.GetType() call is statically unreachable, this could be trimmed */)] | |||
There was a problem hiding this comment.
Could you file an issue for this and link it here?
There was a problem hiding this comment.
Done, filed #110563.
| @@ -1616,7 +1616,7 @@ class AnnotatedInterface | |||
| [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] | |||
| interface IBase | |||
| { | |||
| [Kept] | |||
| [Kept (By = Tool.Trimmer /* interface method is not target of reflection */)] | |||
There was a problem hiding this comment.
Is this something that should be fixed in ILLink? If so, mind filing an issue?
There was a problem hiding this comment.
Ah, so looking at this, this is a bug. Good catch. I thing I got confused by the RequirePublicMethods below, thinking the annotation was just PublicMethods (made the same mistake right now as I was trying to repro).
This should be kept because the actual annotation is .All, not just PublicMethods. We have an existing bug for this so I linked it here.
Progress towards #82447.