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.

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

[release/8.0] Revert "Workaround a C++/CLI bug involving DIMs (#89253)" #93679

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 18, 2023

Backport of #92234 to release/8.0

/cc @jeffhandley @tannergooding

Customer Impact

Earlier during .NET 8, we implemented Ensure that INumberBase implements IUtf8SpanFormattable by tannergooding · Pull Request #88840 · dotnet/runtime as part of introducing IUtf8SpanFormattable, implementing that interface on INumberBase. This implementation introduced a pattern for default interface methods that we hadn't previously employed, and it surfaced a C++/CLI issue that ultimately blocked builds of dotnet/wpf.

To unblock the dotnet/wpf builds, we merged Workaround a C++/CLI bug involving DIMs by tannergooding · Pull Request #89253 · dotnet/runtime and prepared the Revert "Workaround a C++/CLI bug involving DIMs (#89253)" by tannergooding · Pull Request #92234 · dotnet/runtime PR as well.

The C++/CLI issue was fixed in VS 17.8.0 Preview 2, which is now rolled out into our engineering systems and underlies the dotnet/wpf builds. With that in place, we've merged #92234 and this backports that revert into release/8.0, ensuring we have the desired API shape within the .NET 8 release.

Testing

As part of preparing #92234, we verified that the C++/CLI issue is fixed in VS 17.8.0 Preview 2 and our default interface method approach does not cause build breaks for dotnet/wpf.

Risk

Low. We did not observe anything else having trouble with this interface implementation approach. But we will monitor code flow for both main and release/8.0.

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Oct 18, 2023

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #92234 to release/8.0

/cc @jeffhandley @tannergooding

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Numerics, new-api-needs-documentation

Milestone: -

@jeffhandley jeffhandley added Servicing-approved Approved for servicing release and removed new-api-needs-documentation labels Oct 18, 2023
@carlossanlop carlossanlop merged commit 74c81be into release/8.0 Oct 18, 2023
174 checks passed
@carlossanlop carlossanlop deleted the backport/pr-92234-to-release/8.0 branch October 18, 2023 23:38
@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants