Skip to content

Change the search of local function to non-recursive version#63338

Closed
Cosifne wants to merge 2 commits intodotnet:mainfrom
Cosifne:dev/shech/non-recusiveLocalFunctionSearch
Closed

Change the search of local function to non-recursive version#63338
Cosifne wants to merge 2 commits intodotnet:mainfrom
Cosifne:dev/shech/non-recusiveLocalFunctionSearch

Conversation

@Cosifne
Copy link
Copy Markdown
Member

@Cosifne Cosifne commented Aug 11, 2022

Potential fix for https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1582837
@ryzngard and @dibarbet investigate this and it looks like caused by the stackoverflow in the recursive function.

@Cosifne Cosifne requested a review from a team as a code owner August 11, 2022 18:33
@ghost ghost added the Area-IDE label Aug 11, 2022
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Can we have a test for this?

@Cosifne
Copy link
Copy Markdown
Member Author

Cosifne commented Aug 12, 2022

Can we have a test for this?

Fred and David B have done a great job to get a code sample to reproduce this. And I am preparing the test

@Cosifne
Copy link
Copy Markdown
Member Author

Cosifne commented Aug 12, 2022

Update for the test:
I try to test this for a while, but stack overflow is not something easy to repro in unit.
And there is another stackover exception crashes the devenv https://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs,f461f8abcebe1530,references
As long as this issue is resolved we should be able to cover this in integration test.
#63349

@dibarbet
Copy link
Copy Markdown
Member

@Cosifne once #63372 goes in you should be able to cherry-pick this change on top of a revert to the original revert.

@Cosifne
Copy link
Copy Markdown
Member Author

Cosifne commented Aug 18, 2022

This is checked in the other PR.

@Cosifne Cosifne closed this Aug 18, 2022
@Cosifne Cosifne deleted the dev/shech/non-recusiveLocalFunctionSearch branch August 18, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants