Enable BSTR Field Marshaller for x-plat #20264
Conversation
62a4147
to
3e6d761
Compare
|
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test please |
|
@dotnet-bot test Windows_NT arm Cross Debug Innerloop Build please |
|
@dotnet-bot test Ubuntu16.04 arm64 Cross Checked Innerloop Build and Test please |
|
@jashook, Would you take a look changes in src/vm/callingconvention.h and src/vm/methodtable.cpp |
There was a problem hiding this comment.
It seems a little weird to remove the BSTR from the FEATURE_COMINTEROP definitions. It seems like it naturally is included in that definition. I understand you are removing it to not enable FEATURE_COMINTEROP cross plat; however, I wonder if it is better to add another define, maybe FEATURE_COMINTEROP_XPLAT or FEATURE_COMINTEROP_WINDOWS for windows specific com_interop. Either way, it is not my expertise so I can let others give their own opinions.
Other than that the change lgtm
| @@ -1016,7 +1016,7 @@ int ArgIteratorTemplate<ARGITERATOR_BASE>::GetNextOffset() | |||
| case ELEMENT_TYPE_VALUETYPE: | |||
| { | |||
| #ifdef UNIX_AMD64_ABI | |||
| MethodTable *pMT = m_argTypeHandle.AsMethodTable(); | |||
| MethodTable *pMT = m_argTypeHandle.GetMethodTable(); | |||
There was a problem hiding this comment.
@luqunl can you please educate me on why was this change needed?
There was a problem hiding this comment.
For some cases, m_argTypeHandle will point to typepamaterdesc instead of methodtable.
There was a problem hiding this comment.
I saw this comment in typehandle.h
// Unlike AsMethodTable, GetMethodTable will get the method table
// of the type, regardless of whether it is an array etc. Note, however
// this method table may be shared, and some types (like TypeByRef), have
// no method table (and this function returns NULL for them)
inline PTR_MethodTable GetMethodTable() const;
It feels ok to me. I see the BSTR as just another type, not necessarily COM interop specific. |
Yes, We just treat BSTR as a normal 'well know' type which works x-plat. |
# Conflicts: # src/vm/callingconvention.h # tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs # tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp # tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs
* Enable BSTR Marshaling Support for x-plat PInvoke (#19766) # Conflicts: # src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs # src/System.Private.CoreLib/src/System/String.CoreCLR.cs # src/System.Private.CoreLib/src/System/StubHelpers.cs * Enable BSTR Field Marshaller for x-plat (#20264) # Conflicts: # src/vm/callingconvention.h # tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs # tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp # tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs
* Enable BSTR Marshaling Support for x-plat PInvoke (dotnet#19766) # Conflicts: # src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs # src/System.Private.CoreLib/src/System/String.CoreCLR.cs # src/System.Private.CoreLib/src/System/StubHelpers.cs * Enable BSTR Field Marshaller for x-plat (dotnet#20264) # Conflicts: # src/vm/callingconvention.h # tests/src/Interop/StringMarshalling/BSTR/BSTRTest.cs # tests/src/Interop/StringMarshalling/BSTR/BSTRTestNative.cpp # tests/src/Interop/StringMarshalling/BSTR/PinvokeDef.cs
Enable Marshalling BSTR as field in PInvoke for x-plat