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

Always check a return expression in a generator #20621

Merged
1 commit merged into from
Dec 12, 2017
Merged

Conversation

ghost
Copy link

@ghost ghost commented Dec 11, 2017

Fixes #20587
#20326 accidentally changed it so return expressions in generators were never checked. This PR cleans up the control flow so we will still check the expression (though we don't check its type -- there's a TODO comment but is there an issue for that?).

@ghost ghost force-pushed the checkGeneratorReturn branch from 86cadee to d021bce Compare December 11, 2017 17:16
@ghost ghost requested a review from sandersn December 11, 2017 17:48
@Jessidhia
Copy link

There is #18726 for checking the return type at all. That also links to a PR but it seems to be conflicting now.

@ghost
Copy link
Author

ghost commented Dec 12, 2017

@Kovensky thanks

// A generator does not need its return expressions checked against its return type.
// Instead, the yield expressions are checked against the element type.
// TODO: Check return expressions of generators when return type tracking is added
// TODO: Check return types of generators when return type tracking is added
Copy link
Member

Choose a reason for hiding this comment

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

👍

@ghost ghost merged commit d53af09 into master Dec 12, 2017
@ghost ghost deleted the checkGeneratorReturn branch December 12, 2017 15:50
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants