Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move COM ILLink.Descriptors entries to CoreCLR #38172

Merged
merged 1 commit into from
Jun 20, 2020
+8 −8
Merged

Move COM ILLink.Descriptors entries to CoreCLR #38172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/coreclr/src/System.Private.CoreLib/ILLinkTrim.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,13 @@
<type fullname="System.Diagnostics.Tracing.EventPipe*" />
<!-- The private Event methods are accessed by private reflection in the base EventSource class. -->
<type fullname="System.Diagnostics.Tracing.NativeRuntimeEventSource" />
<!-- Accessed via native code. -->
<type fullname="System.Runtime.InteropServices.ComTypes.IEnumerable" />
<type fullname="System.Runtime.InteropServices.ComTypes.IEnumerator" />
<type fullname="System.Runtime.InteropServices.CustomMarshalers.*" />
<!-- Workaround for https://github.com/mono/linker/issues/378 -->
<type fullname="System.Runtime.InteropServices.IDispatch" />
<type fullname="System.Runtime.InteropServices.IMarshal" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this type does exist on mono when '$(TargetsWindows)' == 'true'.

@jkotas @AaronRobinsonMSFT - do you think this is the right decision? Or should I keep it in the "shared" ILLInkTrim since that is where the type exists?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IMarshal should only be used for the StandardOleMarshalObject which is for COM only. Since mono doesn't support COM at all, I think any usage of StandardOleMarshalObject on the mono runtime is technically unsupported. Therefore keeping it in coreclr is probably fine even though usage in a mono scenario would be broken. @jkotas may have another opinion but I think the risk is minimal for user impact.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The support for Mono on Windows overall is incomplete in multiple ways. We are not shipping Mono as supported product (in .NET 5 at least).

Mono CoreLib does not ever define FeatureComInterop and so it gets stubs for other COM interop parts too: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems#L1605

A potential alternative would be to put this under FeatureComInterop instead of Windows/Unix, but I do not think it would be material improvement. There are number of other places in libraries that depend on COM interop to work on Windows.

<type fullname="Internal.Runtime.InteropServices.IClassFactory2" />
</assembly>
</linker>
8 changes: 0 additions & 8 deletions src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Shared.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@
<!-- Methods is used by VS Tasks Window. -->
<method name="GetActiveTaskFromId" />
</type>
<!-- Accessed via native code. -->
<type fullname="System.Runtime.InteropServices.ComTypes.IEnumerable" />
<type fullname="System.Runtime.InteropServices.ComTypes.IEnumerator" />
<type fullname="System.Runtime.InteropServices.CustomMarshalers.*" />
<!-- Workaround for https://github.com/mono/linker/issues/378 -->
<type fullname="System.Runtime.InteropServices.IDispatch" />
<type fullname="System.Runtime.InteropServices.IMarshal" />
<type fullname="Internal.Runtime.InteropServices.IClassFactory2" />
<type fullname="System.Threading.ThreadPoolBoundHandle">
<!-- Workaround to keep .interfaceimpl even though this type
is not instantiated on unix:
Expand Down