Skip to content

Semicolon does not parse in match types #13331

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

Closed
neko-kai opened this issue Aug 18, 2021 · 12 comments · Fixed by #13338
Closed

Semicolon does not parse in match types #13331

neko-kai opened this issue Aug 18, 2021 · 12 comments · Fixed by #13338

Comments

@neko-kai
Copy link
Contributor

Compiler version

3.0.1

Minimized code

def Choice = (_: Any) match { case Int => Long; case Long => Int } // ok

type Choice[A] = A match { case Int => Long ; case Long => Int } // error

https://scastie.scala-lang.org/popPHRHGRTmfZ9n41Un3fQ

Output

'}' expected, but ';' found

Illegal start of toplevel definition

Expectation

Expected the semicolon to separate match type cases the same as for value matches. Couldn't find a previous discussion on this, so I assume this is not intended?

@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

Looks like a bug, yes.

@dwijnand dwijnand added area:parser itype:bug good first issue Perfect for someone who wants to get started contributing and removed itype:bug labels Aug 19, 2021
@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

Correction. The error is according to the syntax. It's

MatchType         ::=  InfixType `match` <<< TypeCaseClauses >>>
TypeCaseClauses   ::=  TypeCaseClause { TypeCaseClause }
TypeCaseClause    ::=  ‘case’ InfixType ‘=>’ Type [nl]

No room for semicolon here. Why does it work for term match? We have:

MatchClause       ::=  ‘match’ <<< CaseClauses >>>
CaseClauses       ::=  CaseClause { CaseClause }
CaseClause        ::=  ‘case’ Pattern [Guard] ‘=>’ Block

Here it's the Block that can end in a semicolon, which you can verify by looking at the rules of https://docs.scala-lang.org/scala3/reference/syntax.html.

Generally, it's a good idea to consult that syntax before speculating whether something is a bug or not. That document is the final arbiter to decide whether something is correct or not. If we decide to change a parsing behavior we also have to change that document, and often suggested changes fail not because we could not hack the parser but because we cannot come up with a clear context free syntax to describe them.

The syntax document is not perfect, but the more eyes are looking at it the better it will get.

@odersky odersky added itype:language enhancement and removed itype:bug area:parser good first issue Perfect for someone who wants to get started contributing labels Aug 19, 2021
@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

So the real question is: Should we change the language to allow this. Which means: Make the decision that this is the right thing to do, change the syntax document, and change the parser. I leave that to others as an exercise.

@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

To come back to the example: The idiomatic style is to leave semicolons out in both cases. But maybe the "least surprises" strategy would be to allow it in both cases since we cannot forbid it in the term match case.

@neko-kai
Copy link
Contributor Author

But maybe the "least surprises" strategy would be to allow it in both cases since we cannot forbid it in the term match case.

I agree, but I was also surprised because I've had an intuition that semicolons and newlines were usually interchangeable as separators. The semicolon in a term match being a property of the block and not a separator on its own wasn't something I thought about.

The idiomatic style is to leave semicolons out in both cases.

I agree, but the reason I've tried this is because that's the only way to write a match in one line in REPL.

@dwijnand
Copy link
Member

Here it's the Block that can end in a semicolon, which you can verify by looking at the rules of docs.scala-lang.org/scala3/reference/syntax.html.

I actually can't verify that, as it looks like the last BlockResult can be preceded by semicolon-terminated statements, but the BlockResult can't end with a semicolon?

In my mental model a semi-colon is like an escaped newline, and clearly newlines are allowed between case clauses.

The idiomatic style is to leave semicolons out in both cases.

I'm surprised you consider it idiomatic because I think it's little known and used, and probably considered confusing and unreadable to not separate cases with a semicolon. Now that I know it's possible I can't help myself from using the short-hand in one-line REPL snippets, but I think I avoid doing so in committed code. I was even considering suggesting to avoid the confusing in match types..

@dwijnand
Copy link
Member

The idiomatic style is to leave semicolons out in both cases.

I agree, but the reason I've tried this is because that's the only way to write a match in one line in REPL.

?

scala> def Choice = (_: Any) match { case Int => Long case Long => Int }
def Choice: Any => AnyValCompanion

scala> type Choice[A] = A match { case Int => Long case Long => Int }

@neko-kai
Copy link
Contributor Author

@dwijnand

I had no idea that would parse until right now 😀

@dwijnand
Copy link
Member

Well if Kai doesn't even know about it, how idiomatic can it really be? 😄

@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

I actually can't verify that, as it looks like the last BlockResult can be preceded by semicolon-terminated statements, but the BlockResult can't end with a semicolon?

But the BlockResult is itself optional.

@odersky
Copy link
Contributor

odersky commented Aug 19, 2021

Well if Kai doesn't even know about it, how idiomatic can it really be? 😄

Fair point 😄 . I think it does not hurt to allow the ; in this case.

odersky added a commit to dotty-staging/dotty that referenced this issue Aug 19, 2021
@som-snytt
Copy link
Contributor

I leave that to others as an exercise.

This is one of those exercises where they give you the answer at the bottom.

Scala 2 has a mix of styles, noting Adriaan prefers two spaces, but the use case is for default case:

x match { case C(_) => true case _ => false }

for which the alternative is

PartialFunction.cond(x) { case C(_) => true }

or similar. So maybe the style question is about canonical expression of default case.

olsdavis pushed a commit to olsdavis/dotty that referenced this issue Apr 4, 2022
@Kordyjan Kordyjan added this to the 3.1.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants