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

Alternative fix for RuntimeHelpers.GetSubArray AOT compatibility warning #95296

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Nov 27, 2023

C# LDM would like to keep the original "virtual" RuntimeHelpers.GetSubArray behavior.

Fixes dotnet/roslyn#69053

@ghost
Copy link

ghost commented Nov 27, 2023

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

Issue Details

C# LDM would like to keep the original "virtual" RuntimeHelpers.GetSubArray behavior.

Fixes dotnet/roslyn#69053

Author: jkotas
Assignees: jkotas
Labels:

area-System.Runtime.CompilerServices

Milestone: -

@jkotas jkotas added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Nov 27, 2023
@ghost ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Nov 27, 2023
@ghost
Copy link

ghost commented Nov 27, 2023

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@jkotas
Copy link
Member Author

jkotas commented Nov 27, 2023

@jcouv Here is revert of dotnet/roslyn#69053 (comment) .

I think we will need to file a breaking notice for this change. This can both:

  • Change the behavior of complex pattern matching expression
  • Break code that expects the array slicing to return the "non-virtual" type. For example:
test(new string[10]);

static object test(object[] a)
{
    var b = a[1..2];
    b[0] = new object();
    return b;
}

I will need your help with providing the justification.

@stephentoub
Copy link
Member

I think we will need to file a breaking notice for this change.

This was introduced in .NET 7, right? Should we consider backporting it to .NET 8 asap, such that effectively it would only impact folks that were on .NET 7 or early .NET 8 adopters?

@jkotas
Copy link
Member Author

jkotas commented Nov 27, 2023

This was introduced in .NET 7, right?

Right.

Should we consider backporting it to .NET 8 asap, such that effectively it would only impact folks that were on .NET 7 or early .NET 8 adopters?

It is a corner case and fixing it is a potential breaking change. We have lived with the current behavior for 1.5 years. I do not think that backporting this change meets the servicing bar.

@jkotas jkotas merged commit 66aeb98 into dotnet:main Nov 29, 2023
178 checks passed
@jkotas jkotas deleted the roslyn-69053 branch November 29, 2023 03:54
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.CompilerServices breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Slice pattern with type pattern generates redundant type test (or fails to perform the test intended)
4 participants