Convert OAVariant interop to managed#100176
Conversation
|
As my understanding there are two flavors of Variant: COM Variant and OLE Variant. |
|
Well COM uses OLE VARIANT(System.Runtime.InteropServices.Marshalling.ComVariant), System.Variant/VariantData/CVType are CLR only intermediate representations. We should favor ComVariant and avoid the System.Variant layer as possible. |
|
A relevant bug (?) discovered: runtime/src/coreclr/vm/olevariant.h Lines 53 to 60 in c659c6e In managed OAVariantLib, it lacks a slot for CURRENCY. So probably if you call This comes from .NET Framework and probably we won't ever fix it without refactoring 😆 |
|
I think it's at a reasonable step point now. All |
Nice. These are great learnings as I don't think many of us have given much thought into how far these systems have drifted from when they were originally written. Do you have any thoughts on what this new information means for how to test and/or how to think about the utility of this code? |
|
For this PR, the cases are now narrowed, because the possible inputs are now limited to anything returned from I still think we need a standalone PR for test, and possibly compare the behavior with .NET Framework since it will be more correct. The test should focus on the effect of this code: what level of type mismatch are allowed in parameters, ref and returns. What are rejected causing by this code should also be covered. The test shouldn't be exhausting the conversion matrix, nor testing the behavior of |
|
I've cleaned up a bit based on tests in #101841. Only possible types from Requesting for another round of review. |
src/coreclr/System.Private.CoreLib/src/Microsoft/Win32/OAVariantLib.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/Microsoft/Win32/OAVariantLib.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/Microsoft/Win32/OAVariantLib.cs
Outdated
Show resolved
Hide resolved
AaronRobinsonMSFT
left a comment
There was a problem hiding this comment.
Thanks @huoyaoyuan. Appreciate your perseverance.
* Convert BoxEnum to managed * SetFieldsObject to managed * Handle decimal and other CV_OBJECT * Managed ToOAVariant and FromOAVariant * Marshal for IUnknown and IDispatch * VariantChangeTypeEx interop * Use managed reflection for System.Drawing.Color conversion * Use MarshalNative for IDispatch/IUnknown marshalling * Move Color conversion to Variant * Respect VTToCV mapping * Improve test type coverage * Test for values in ReturnToManaged --------- Co-authored-by: Aaron Robinson <[email protected]>
Still draft because I'm not sure of feature consistency or coverage here.