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

Fix broken 'convert if to switch' case with recursive analysis #76317

Merged

Conversation

CyrusNajmabadi
Copy link
Member

Fixes #71295

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner December 7, 2024 01:23
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 7, 2024
@CyrusNajmabadi
Copy link
Member Author

@alrz can you let me know if this meets your expectations here?

Comment on lines +117 to +122
// If we're in the initial state, it's fine for there to be many operations that follow. We're just
// trying to check if the first one completes our analysis (and we'll not touch the ones that
// follow). However, if we're actually in one of the recursive calls, it's *not* ok to ignore the
// following ops as those may impact if the higher call into us is ok. So in that case, we do not
// allow the parsing to succeed if we have more than one operation left.
return topLevel
Copy link
Member

Choose a reason for hiding this comment

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

The topLevel distinction makes complete sense. I don't have the mental model on the inner workings of this right now but I might have missed other places where this matters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit 9593ad2 into dotnet:main Dec 10, 2024
25 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the convertIfToSwitchNestedAnalysis branch December 10, 2024 00:43
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 10, 2024
@dibarbet dibarbet modified the milestones: Next, 17.13 P3 Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert to 'switch' statement changes behaviour
4 participants