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

Add support for let chains #11320

Closed
c410-f3r opened this issue Jan 19, 2022 · 8 comments · Fixed by #11375
Closed

Add support for let chains #11320

c410-f3r opened this issue Jan 19, 2022 · 8 comments · Fixed by #11375
Labels
A-parser parser issues S-actionable Someone could pick this issue up and work on it right now

Comments

@c410-f3r
Copy link
Contributor

See rust-lang/rust#88642.

@bjorn3 bjorn3 added the A-parser parser issues label Jan 19, 2022
@Veykril Veykril added the S-actionable Someone could pick this issue up and work on it right now label Jan 19, 2022
@ChayimFriedman2
Copy link
Contributor

I would like to work on that. How should I synchronize the changes to ungrammer and rust-analyzer?

@ChayimFriedman2
Copy link
Contributor

Parsing turns out to be quite complex. The problem is that the let syntax can appear inside parentheses (if (let pat = expr)). This is to facilitate for macros, as stated in the RFC.

The problem is especially tricky since we cannot know if let is allowed in this position until we finish parsing it, becauses parentheses can be also tuples: if (let true = true, false) {} is invalid, but if (let true = true) {} is valid.

rustc chose to implement this as an expression kind and later verify that let is indeed valid there.

We don't have a validation pass to the AST. But it may be necessary to introduce it.

What do you think? Do you have an other way?

@Veykril
Copy link
Member

Veykril commented Jan 21, 2022

Macros strike once more, way to go making the language a bit more inconsistent yet again 🙄

We did have a validation pass but disabled it for the time being(I don't recall the reason). If we don't reject the let expression for now where its disallowed that is fine.

@TennyZhuang
Copy link

Is it possible to implement let without parentheses first? It’s the main usecase, and the macro is corner case.

@ChayimFriedman2
Copy link
Contributor

ChayimFriedman2 commented Jan 23, 2022

@TennyZhuang This is quite useless since if you want to implement it using the exisiting infra you will need to rewrite it anyway and if you want to create let expressions, like I did, it is almost no work to support parentheses. I'm also pretty close to closing this, hope for today.

@ChayimFriedman2
Copy link
Contributor

ChayimFriedman2 commented Jan 24, 2022

I've finished the work (I think), but I cannot send a PR until rust-analyzer/ungrammar#45 is merged and published since Cargo can't update Cargo.lock until then.

@c410-f3r
Copy link
Contributor Author

Thank you @ChayimFriedman2

@ChayimFriedman2
Copy link
Contributor

After I implemented all of this, now the Rust developers want to forbid parentheses in let chains! 😭

At least we can issue a better error... Though I am not sure parsing it is required to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser parser issues S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants