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

Remove tail recursion and enable cancellation #3229

Merged
merged 1 commit into from
Jun 3, 2015

Conversation

mattwar
Copy link
Contributor

@mattwar mattwar commented Jun 1, 2015

Fixes devdiv 1089540

The problem was a stack overflow in the formatter when pasting a file with a enormous array initializer. The culprit was a method FormattingContext.GetEndTokenForRelativeIndentationSpan that was making a recursive call after looking forward to the next token, attempting to find the end token for a relative indentation operation. Fortunately it was simply tail recursive and was easy to rewrite as a loop.

Also noticed that the whole operation when successful was taking tremendous amount of time (on order of 10 minutes), and was stuck in this recursive method for a long time without ability to cancel, so I plumbed cancellation through to its loop.

Also noticed that the dialog that pops up on slow execution of formatting did not have a cancel button, so I enabled that too.

@heejaechang @jasonmalinowski @Pilchie please review.

@jasonmalinowski
Copy link
Member

More reviewers: @DustinCampbell, @balajikris, @dpoeschl@, @brettfo

@Pilchie
Copy link
Member

Pilchie commented Jun 1, 2015

👍

@dpoeschl
Copy link
Contributor

dpoeschl commented Jun 1, 2015

retest this please

@Pilchie
Copy link
Member

Pilchie commented Jun 2, 2015

retest this please

@DustinCampbell
Copy link
Member

👍

// reached end of tree
return default(SyntaxToken);
}
// recursively find the end token outside of inseparable regions
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the word "recursively"?

@pharring
Copy link
Contributor

pharring commented Jun 2, 2015

👍

@MattGertz
Copy link
Contributor

Approved 6/3 by ML Shiproom.

mattwar added a commit that referenced this pull request Jun 3, 2015
Remove tail recursion and enable cancellation
@mattwar mattwar merged commit 09aac2c into dotnet:stabilization Jun 3, 2015
@mattwar mattwar deleted the Bug1089540 branch August 29, 2016 19:50
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.

8 participants