[TrimmableTypeMap][Core B] Foundation and AssemblyIndex#10817
Conversation
| /// <summary> | ||
| /// Parsed [Register] or [Export] attribute data for a type or method. | ||
| /// </summary> | ||
| sealed class RegisterInfo |
There was a problem hiding this comment.
We should consider splitting this into RegisterInfo and ExportInfo
| if (attrName == "RegisterAttribute") { | ||
| registerInfo = ParseRegisterAttribute (ca, customAttributeTypeProvider); | ||
| } else if (attrName == "ExportAttribute") { | ||
| // [Export] methods are detected per-method in CollectMarshalMethods |
There was a problem hiding this comment.
| // [Export] methods are detected per-method in CollectMarshalMethods | |
| // [Export] methods are not handled yet and supporting them wil be implemented later |
| foreach (var caHandle in typeDef.GetCustomAttributes ()) { | ||
| var ca = Reader.GetCustomAttribute (caHandle); |
There was a problem hiding this comment.
| foreach (var caHandle in typeDef.GetCustomAttributes ()) { | |
| var ca = Reader.GetCustomAttribute (caHandle); | |
| var attributes = typeDef.GetCustomAttributes ().Select (Reader.GetCustomAttribute); | |
| foreach (var caHandle in attributes) { |
| if (typeDef.IsNested) { | ||
| var declaringType = reader.GetTypeDefinition (typeDef.GetDeclaringType ()); | ||
| var parentName = GetFullName (declaringType, reader); | ||
| return parentName + "+" + name; |
There was a problem hiding this comment.
| return parentName + "+" + name; | |
| return $"{parentName}+{name}"; |
| return name; | ||
| } | ||
|
|
||
| return ns + "." + name; |
There was a problem hiding this comment.
| return ns + "." + name; | |
| return $"{ns}.{name}"; |
|
|
||
| public static AssemblyIndex Create (string filePath) | ||
| { | ||
| var peReader = new PEReader (File.OpenRead (filePath)); |
There was a problem hiding this comment.
Do we need to consider IO errors here? Or will this need to be handled by the user of this class?
| /// <summary> | ||
| /// Type has [Activity], [Service], [BroadcastReceiver], [ContentProvider], | ||
| /// [Application], or [Instrumentation]. | ||
| /// </summary> | ||
| public bool HasComponentAttribute { get; set; } |
There was a problem hiding this comment.
I don't really know if this information would be useful later, but it would be useful in tests I think: let's remember the exact type of the attribute please. I think it might be best to make this class abstract and add a sealed subclass for each of the component classes. The ApplicationBackupAgent and ApplicationManageSpaceActivity fields should be just on the ApplicationAttributeInfo class (without the redundant Application prefix).
|
|
||
| bool ImplementsIJniNameProviderAttribute (TypeDefinition typeDef) | ||
| { | ||
| foreach (var implHandle in typeDef.GetInterfaceImplementations ()) { |
There was a problem hiding this comment.
Does typeDef.GetInterfaceImplementations () return only the interfaces implemented on this type level, or also the inherited interfaces? I would expect this API not to recursively explore the interfaces implemented on the base class and the base interfaces of the implemented interfaces. I think we should either not do this yet (we would only support the predefined set of attributes such as [Register], [Application], [Activity], ..., or do it properly and recursively explore the structure. This could be very expensive, so we'd need to carefuly cache if a given type (class or interface) implements IJniNameProviderAttribute. The more I'm thinking about it, the more I think we should not even look for this interface. We can add this logic once we run into a scenario that actually requires it.
|
|
||
| var (registerInfo, attrInfo) = ParseAttributes (typeDef); | ||
|
|
||
| if (attrInfo != null) { |
There was a problem hiding this comment.
| if (attrInfo != null) { | |
| if (attrInfo is not null) { |
| continue; | ||
| } | ||
|
|
||
| TypesByFullName [fullName] = typeHandle; |
There was a problem hiding this comment.
Wouldn't it be better to store the resolved typeDef? It might not be, I'm just curious why we're storing the handle and not the actual definition in this cache.
|
Addressed all review feedback in this and downstream stacked PRs: applied null patterns ( |
Sliced from #10805
Depends on #10816
Scope
project, model/providers)AssemblyIndexmetadata indexing and lookup logicNotes