Skip to content

Use synthesized methods for compiler-generated switch exceptions#59975

Merged
jcouv merged 7 commits intodotnet:mainfrom
Youssef1313:59601-switch-throw-wrapper
Mar 25, 2022
Merged

Use synthesized methods for compiler-generated switch exceptions#59975
jcouv merged 7 commits intodotnet:mainfrom
Youssef1313:59601-switch-throw-wrapper

Conversation

@Youssef1313
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 commented Mar 6, 2022

Fixes #59601

@RikkiGibson Tagging you since you self-assigned the issue.

@Youssef1313 Youssef1313 requested a review from a team as a code owner March 6, 2022 07:40
@ghost ghost added Community The pull request was submitted by a contributor who is not a Microsoft employee. Area-Compilers labels Mar 6, 2022
@Youssef1313

This comment was marked as resolved.

Sign in to view
@RikkiGibson RikkiGibson self-assigned this Mar 6, 2022
@RikkiGibson

This comment was marked as resolved.

Sign in to view
@Youssef1313 Youssef1313 requested a review from RikkiGibson March 9, 2022 13:16
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 9, 2022

Great progress here. My only concern currently is I don't see tests where:

  1. the parameterless ThrowSwitchExpressionException is used. For example, when switch input is a pointer type or ref struct.
  2. InvalidOperationException is used. For example, if we use MakeMemberMissing on both SwitchExpressionException constructors.

@Youssef1313
Copy link
Copy Markdown
Member Author

Youssef1313 commented Mar 10, 2022

@RikkiGibson I updated some existing tests.

@runfoapp runfoapp bot mentioned this pull request Mar 10, 2022
Closed
@RikkiGibson RikkiGibson requested a review from a team March 10, 2022 17:14
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 10, 2022

@dotnet/roslyn-compiler for second review.

@jcouv jcouv self-assigned this Mar 17, 2022
jcouv
jcouv reviewed Mar 23, 2022
View reviewed changes
jcouv
jcouv reviewed Mar 23, 2022
View reviewed changes
jcouv
jcouv reviewed Mar 23, 2022
View reviewed changes
jcouv
jcouv reviewed Mar 23, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 6). Overall looks good, only minor feedback and coverage recommendations

jcouv
jcouv approved these changes Mar 24, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 7)

@RikkiGibson RikkiGibson enabled auto-merge (squash) March 24, 2022 16:36
This was referenced Mar 24, 2022
Closed
Closed
@RikkiGibson RikkiGibson disabled auto-merge March 25, 2022 18:28
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 25, 2022

/azp run roslyn-integration-ci

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 25, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 25, 2022

/azp run roslyn-integration-corehost

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 25, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@jcouv jcouv enabled auto-merge (squash) March 25, 2022 18:45
@jcouv jcouv merged commit 5a8dc97 into dotnet:main Mar 25, 2022
@ghost ghost added this to the Next milestone Mar 25, 2022
@Youssef1313 Youssef1313 deleted the 59601-switch-throw-wrapper branch March 25, 2022 20:01
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@RikkiGibson RikkiGibson

@jcouv jcouv

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Milestone

17.2.P3

Development

Successfully merging this pull request may close these issues.

Use synthesized wrapper methods for compiler-generated 'throw's

4 participants

@Youssef1313 @RikkiGibson @jcouv @allisonchou