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

[Minor breaking] Allow named arguments in commands and streamline type expressions #19383

Closed
wants to merge 17 commits into from

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Jan 13, 2022

Closes nim-lang/RFCs#442, closes #15050, closes #8846 and maybe some other issues, cleans up after #19181, closes #19313.

Types are separated from normal expressions in the grammar.

object, tuple etc are special in type sections and cannot directly be used as expressions. This gives errors similar to #19313 for objects and tuples (which is reflected by the ordered choice in the grammar) and is fixed by wrapping in parentheses. ref and ptr (and also distinct because a package was using distinct object) are also special, but this is only when object and tuple immediately follow.

foo a = b previously meant foo(a) = b, now it means foo(a = b). This only applies to statement form expressions, exprStmt in the grammar, although this is not reflected in the grammar yet as it would be pretty complicated. Currently it uses a PrimaryMode enum field instead. Allowing this also resulted in line info of commands being more accurate, and commands now terminate on dedented commas which was not the case before.

Both of these are (minorly) breaking. Changing code to make these work is very easy, however the second one will not give a direct syntax error upon being wrong and the first one gives errors that were not previously given. For this reason it can be delayed. #19191 and #19181 also had parser changes, but these added new syntax and were non-breaking.

@metagn metagn changed the title Breaking parser changes [Breaking] Allow named arguments in commands and streamline type expressions Jan 14, 2022
@metagn metagn mentioned this pull request Jan 14, 2022
compiler/parser.nim Outdated Show resolved Hide resolved
compiler/parser.nim Outdated Show resolved Hide resolved
compiler/semtypes.nim Outdated Show resolved Hide resolved
@metagn
Copy link
Collaborator Author

metagn commented Jan 14, 2022

PR is pretty much done with some minor things listed above. Leaving it as draft until it's safe to merge breaking changes like this.

@Araq
Copy link
Member

Araq commented Jan 19, 2022

It's hard to foresee the effects this has on existing code. I do like the changes though. But it's time to derive the parser from a formal grammar, we want to encourage alternative implementations and better editor plugins.

@metagn
Copy link
Collaborator Author

metagn commented Jan 19, 2022

What's the difference with normal PEG? Indents?

@Araq
Copy link
Member

Araq commented Jan 20, 2022

Yes.

@metagn metagn marked this pull request as ready for review July 10, 2022 20:09
@metagn metagn changed the title [Breaking] Allow named arguments in commands and streamline type expressions Allow named arguments in commands and streamline type expressions Jul 10, 2022
@metagn metagn changed the title Allow named arguments in commands and streamline type expressions [Minor breaking] Allow named arguments in commands and streamline type expressions Jul 10, 2022
@metagn
Copy link
Collaborator Author

metagn commented Jul 10, 2022

Decided to unmark as draft considering how rarely breaking this is. Any choice is fine with me.

@Varriount Varriount requested a review from Araq August 31, 2022 18:48
@Varriount Varriount added the Requires Araq To Merge PR should only be merged by Araq label Aug 31, 2022
@metagn metagn force-pushed the parser-grammar branch 3 times, most recently from 6395a7c to d5dc3fb Compare September 7, 2022 17:41
@metagn
Copy link
Collaborator Author

metagn commented Sep 17, 2022

When will this be reviewed?

@metagn
Copy link
Collaborator Author

metagn commented Nov 26, 2022

Bump, this would be really nice to have before new parser PRs

@metagn
Copy link
Collaborator Author

metagn commented Dec 2, 2022

moved to #20994

@metagn metagn closed this Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires Araq To Merge PR should only be merged by Araq
Projects
None yet
3 participants