Skip to content

Conversation

sasurau4
Copy link
Contributor

@sasurau4 sasurau4 commented Jan 8, 2021

Part of #4309
The inference of yield will be implemented at another PR.

@Veykril
Copy link
Member

Veykril commented Jan 8, 2021

Those files are auto-generated by xtask. To support yield we have to add a YieldExpr rule to the ungrammar file and the Expr rule, have a new version published, then regenerate these files here and adjust the parser.

bors bot added a commit to rust-analyzer/ungrammar that referenced this pull request Jan 12, 2021
19: Add YieldExpr r=lnicola a=sasurau4

Adding YieldExpr

Part of rust-lang/rust-analyzer#7209
ref: https://doc.rust-lang.org/beta/unstable-book/language-features/generators.html

Co-authored-by: Daiki Ihara <sasurau4@gmail.com>
@lnicola
Copy link
Member

lnicola commented Jan 12, 2021

ungrammar 1.7 is out.

@Veykril
Copy link
Member

Veykril commented Jan 12, 2021

As a heads up since you haven't touched that part here before either, to make the parser support the yield syntax you will have to add the parsing for it at the correct places in the https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/parser/src/grammar/expressions/atom.rs module.

@sasurau4 sasurau4 force-pushed the feature/add-generator-keywords branch 2 times, most recently from 38550ac to d2fb5a5 Compare January 13, 2021 15:05
@sasurau4 sasurau4 force-pushed the feature/add-generator-keywords branch from d2fb5a5 to b557ea7 Compare January 14, 2021 11:29
@sasurau4 sasurau4 requested a review from lnicola January 14, 2021 11:29
@sasurau4
Copy link
Contributor Author

@Veykril Thanks for guiding!

Expr::Return { expr } => {
Expr::Return { expr } | Expr::Yield { expr } => {
if let Some(expr) = expr {
self.infer_expr_coerce(*expr, &Expectation::has_type(self.return_ty.clone()));
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't apply to yield I think. A generator may have a return type and a yield type that aren't the same. To properly implement inference for yield we need to track the yield type in the InferenceContext I believe but that can be done in a separate PR I'd say.
So for now we probably want something like this:

Expr::Yield { expr } => {
	  // FIXME: track yield type for coercion
	  if let Some(expr) = expr {
	      self.infer_expr(expr, &Expectation::none());
	  }
	  Ty::simple(TypeCtor::Never)
}

@sasurau4 sasurau4 force-pushed the feature/add-generator-keywords branch from b557ea7 to 138514b Compare January 15, 2021 14:35
@sasurau4 sasurau4 requested a review from Veykril January 15, 2021 14:38
@Veykril
Copy link
Member

Veykril commented Jan 15, 2021

lgtm 👍
bors r+

@bors
Copy link
Contributor

bors bot commented Jan 15, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants