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

Don't optimize large tuple literal switch inputs #48028

Merged
merged 3 commits into from
Sep 24, 2020

Conversation

RikkiGibson
Copy link
Contributor

Fixes #47878

This is a simpler, less optimal fix than #47893. We can try to figure out how to fix the optimization, but I wanted to submit this as a candidate fix for 16.8 in case the optimization is hard to get right for large tuples.

@RikkiGibson RikkiGibson marked this pull request as ready for review September 24, 2020 21:34
@RikkiGibson RikkiGibson requested a review from a team as a code owner September 24, 2020 21:34
{
int x1 = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) switch
{
(1, 2, 3, 4, 5, 6, 7, 8, 9, 10) => 1,
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a negative case here, basically a literal that won't match. Just for completeness.

Copy link
Contributor

@chsienki chsienki left a comment

Choose a reason for hiding this comment

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

LGTM minus a couple of test naming nits

}

[Fact, WorkItem(47878, "https://github.com/dotnet/roslyn/issues/47878")]
public void VerifyIL_8ElementsTuple_SideEffects_DefaultLabel()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: doesn't actually verify IL

}

[Fact, WorkItem(47878, "https://github.com/dotnet/roslyn/issues/47878")]
public void VerifyIL_9ElementsTuple_SideEffects_DefaultLabel()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: VerifyIL

@RikkiGibson
Copy link
Contributor Author

It looks like the integration queue is not running. Will force merge.

@RikkiGibson RikkiGibson merged commit f33a0fd into dotnet:release/dev16.8 Sep 24, 2020
@RikkiGibson RikkiGibson deleted the fix-47878-simple branch September 25, 2020 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants