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

Do not allow a let-statement as an embedded statement. #9216

Merged
merged 1 commit into from
Feb 26, 2016

Conversation

gafter
Copy link
Member

@gafter gafter commented Feb 26, 2016

Do not allow a local function declaration as an embedded statement.
Handle deep expressions better in the pattern variable finder.

@AlekseyTs @CyrusNajmabadi @jaredpar FYI

Do not allow a local function declaration as an embedded statement.
Handle deep expressions better in the pattern variable finder.
@gafter gafter self-assigned this Feb 26, 2016
@gafter gafter added this to the 2.0 (Preview) milestone Feb 26, 2016
@gafter
Copy link
Member Author

gafter commented Feb 26, 2016

This should fix a few test failures due to the pattern variable finder overflowing the stack.

gafter added a commit that referenced this pull request Feb 26, 2016
Do not allow a let-statement as an embedded statement.
@gafter gafter merged commit 903d334 into dotnet:features/patterns Feb 26, 2016
s_poolInstance.Free(finder);
return result;
}

private void VisitExpressions()
{
while (expressionsToVisit.Count != 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is doesn't preserve the order in which expressions are visited, which leads to a wrong behavior when duplicate declarations are involved. Please fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

See how it was done for declaration expressions in LocalScopeBinder.cs.
The implementation of VisitBinaryExpression of interest was removed in 9d36a64a.

@gafter gafter deleted the threeFixes branch May 24, 2018 19:05
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.

3 participants