Add Context checks to factory caching and simplify factory caching between factories and statics. #1367
Conversation
…tween factories and statics.
Sergio0694
left a comment
There was a problem hiding this comment.
Just leaving a couple more comments 😄
| private static extern unsafe int CoGetContextToken(IntPtr* contextToken); | ||
|
|
||
| [DllImport("api-ms-win-core-com-l1-1-0.dll")] | ||
| private static extern int CoGetObjectContext(ref Guid riid, out IntPtr ppv); |
There was a problem hiding this comment.
Same note as in the other PR, could you make this use a blittable signature while at it?
52fb3e5 to
73d5e29
Compare
Sergio0694
left a comment
There was a problem hiding this comment.
Left a few comments 🙂
Also, Q: I see you removed INativeGuid, is that just because it'll all be done in another PR?
| #endif | ||
| public static ObjectReference<I> ActivateInstance< | ||
| #if NET | ||
| [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.None)] |
There was a problem hiding this comment.
Is this actually needed? Isn't I already not annotated for any dynamic access in this context? 🤔
| [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2091:RequiresUnreferencedCode", | ||
| Justification = "No members of the generic type are dynamically accessed in this code path.")] | ||
| #endif | ||
| public unsafe ObjectReference<I> _ActivateInstance<I>() |
There was a problem hiding this comment.
Also double checking: I see you removed the [UnconditionalSuppressMessage] below. Is this one still needed or can it also be removed after the other changes you did?
| % | ||
| internal static % Instance => _instance; | ||
| } | ||
| private static % _% = new %("%.%", %.IID); |
There was a problem hiding this comment.
Is this ever reassigned at runtime? If not, shouldn't this be readonly to improve codegen?
| { | ||
| auto class_type_name = classType.TypeName(); | ||
| w.write(R"( | ||
| private static BaseActivationFactory _%Factory = new BaseActivationFactory("%.%"); |
There was a problem hiding this comment.
Same for this one, shouldn't it be readonly, or could this ever be reassigned?
Alternative implementation that maintains static classes compared to #1359