Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 3, 2021

Backport of #58646 to release/5.0

/cc @AaronRobinsonMSFT

Customer Impact

This enables support for variance in an IDynamicInterfaceCastable scenario. The primary user of this, at present, is C#/WinRT. Without this there can be significant performance impact implementing variance in WinRT collections.

The precise impact of this change is difficult to define since it dependent on managed type and the generated source for a particular application. However, a thought experiment to predict potential impact can be done using IEnumerable<T>. Imagine the case where T is defined to be string. The generated C#/WinRT tools generated a projection for IEnumerable<string> of a particular type. Consider the case where a user requests IEnumerable<object>. Based on the current implementation this would fail because IEnumerable<object> is not IEnumerable<string>. What this means is C#/WinRT would now need to dynamically create an additional wrapper, at runtime, to wrap the IEnumerable<string> projection into a IEnumerable<object>. This fix avoids that because it enables the runtime to apply its own variance checks via IsAssignableTo – which is true based on covariance.

Testing

Tests have been added.

Risk

Minimal

@ghost ghost added the area-Interop-coreclr label Sep 3, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT added the Servicing-consider Issue for next servicing release review label Sep 3, 2021
@AaronRobinsonMSFT
Copy link
Member

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. Please request a code review and we can take for consideration in 5.0.x.

@jeffschwMSFT jeffschwMSFT removed the Servicing-consider Issue for next servicing release review label Sep 8, 2021
@Anipik
Copy link
Contributor

Anipik commented Oct 8, 2021

@jeffschwMSFT do we need to add back the servicing-consider label here for next release ? cc @aik-jahoda

@jeffschwMSFT
Copy link
Member

Thanks for checking. We are not considering this for servicing for now. We are waiting for the e2e to come together.

@jkotas
Copy link
Member

jkotas commented Jan 26, 2022

5.0 is going out of support in a few months.

@jkotas jkotas closed this Jan 26, 2022
@akoeplinger akoeplinger deleted the backport/pr-58646-to-release/5.0 branch February 1, 2022 11:12
@ghost ghost locked as resolved and limited conversation to collaborators Mar 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants