Skip to content

Parsing indentation cleanup #10861

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

Merged
merged 4 commits into from
Jan 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ object Parsers {

def possibleTemplateStart(isNew: Boolean = false): Unit =
in.observeColonEOL()
if in.token == COLONEOL || in.token == WITHEOL then
if in.token == COLONEOL || in.token == WITH then
if in.lookahead.isIdent(nme.end) then in.token = NEWLINE
else
in.nextToken()
Expand Down Expand Up @@ -3545,7 +3545,7 @@ object Parsers {
ValDef(name, parents.head, subExpr())
else
DefDef(name, tparams, vparamss, parents.head, subExpr())
else if in.token != WITH && in.token != WITHEOL && parentsIsType then
else if in.token != WITH && parentsIsType then
if name.isEmpty then
syntaxError(em"anonymous given cannot be abstract")
DefDef(name, tparams, vparamss, parents.head, EmptyTree)
Expand Down Expand Up @@ -3633,12 +3633,12 @@ object Parsers {
/** `{`with` ConstrApp} but no EOL allowed after `with`.
*/
def withConstrApps(): List[Tree] =
if in.token == WITH then
in.observeWithEOL() // converts token to WITHEOL if at end of line
if in.token == WITH && in.lookahead.token != LBRACE then
in.nextToken()
constrApp() :: withConstrApps()
else Nil
def isTemplateStart =
val la = in.lookahead
la.isAfterLineEnd || la.token == LBRACE
if in.token == WITH && !isTemplateStart then
in.nextToken()
constrApp() :: withConstrApps()
else Nil

/** Template ::= InheritClauses [TemplateBody]
Expand Down Expand Up @@ -3708,8 +3708,8 @@ object Parsers {

/** with Template, with EOL <indent> interpreted */
Copy link
Contributor

Choose a reason for hiding this comment

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

should this comment be updated? Is this comment a pseudo grammar? Just from the comment it is not clear which "with" is a keyword and whether "," is a token or an informal alternative.

def withTemplate(constr: DefDef, parents: List[Tree]): Template =
if in.token != WITHEOL then accept(WITH)
possibleTemplateStart() // consumes a WITHEOL token
if in.token != WITH then syntaxError(em"`with` expected")
possibleTemplateStart() // consumes a WITH token
val (self, stats) = templateBody()
Template(constr, parents, Nil, self, stats)
.withSpan(Span(constr.span.orElse(parents.head.span).start, in.lastOffset))
Expand Down
15 changes: 6 additions & 9 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ object Scanners {

def isNestedStart = token == LBRACE || token == INDENT
def isNestedEnd = token == RBRACE || token == OUTDENT

/** Is current token first one after a newline? */
def isAfterLineEnd: Boolean = lineOffset >= 0
}

abstract class ScannerCommon(source: SourceFile)(using Context) extends CharArrayReader with TokenData {
Expand Down Expand Up @@ -512,15 +515,12 @@ object Scanners {
|Previous indent : $lastWidth
|Latest indent : $nextWidth"""

private def switchAtEOL(testToken: Token, eolToken: Token): Unit =
if token == testToken then
def observeColonEOL(): Unit =
if token == COLON then
lookAhead()
val atEOL = isAfterLineEnd || token == EOF
reset()
if atEOL then token = eolToken

def observeColonEOL(): Unit = switchAtEOL(COLON, COLONEOL)
def observeWithEOL(): Unit = switchAtEOL(WITH, WITHEOL)
if atEOL then token = COLONEOL

def observeIndented(): Unit =
if indentSyntax && isNewLine then
Expand Down Expand Up @@ -610,9 +610,6 @@ object Scanners {
}
}

/** Is current token first one after a newline? */
def isAfterLineEnd: Boolean = lineOffset >= 0

/** Is there a blank line between the current token and the last one?
* A blank line consists only of characters <= ' '.
* @pre afterLineEnd().
Expand Down
8 changes: 3 additions & 5 deletions compiler/src/dotty/tools/dotc/parsing/Tokens.scala
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ object Tokens extends TokensCommon {
final val QUOTE = 87; enter(QUOTE, "'")

final val COLONEOL = 88; enter(COLONEOL, ":", ": at eol")
final val WITHEOL = 89; enter(WITHEOL, "with", "with at eol")
final val SELFARROW = 90; enter(SELFARROW, "=>") // reclassified ARROW following self-type
final val SELFARROW = 89; enter(SELFARROW, "=>") // reclassified ARROW following self-type

/** XML mode */
final val XMLSTART = 99; enter(XMLSTART, "$XMLSTART$<") // TODO: deprecate
Expand Down Expand Up @@ -277,8 +276,7 @@ object Tokens extends TokensCommon {
final val closingRegionTokens = BitSet(RBRACE, RPAREN, RBRACKET, CASE) | statCtdTokens

final val canStartIndentTokens: BitSet =
statCtdTokens | BitSet(COLONEOL, WITHEOL, EQUALS, ARROW, LARROW, WHILE, TRY, FOR, IF)
// `if` is excluded because it often comes after `else` which makes for awkward indentation rules TODO: try to do without the exception
statCtdTokens | BitSet(COLONEOL, WITH, EQUALS, ARROW, CTXARROW, LARROW, WHILE, TRY, FOR, IF, THROW)

/** Faced with the choice between a type and a formal parameter, the following
* tokens determine it's a formal parameter.
Expand All @@ -287,7 +285,7 @@ object Tokens extends TokensCommon {

final val scala3keywords = BitSet(ENUM, ERASED, GIVEN)

final val endMarkerTokens = identifierTokens | BitSet(IF, WHILE, FOR, MATCH, TRY, NEW, GIVEN, VAL, THIS)
final val endMarkerTokens = identifierTokens | BitSet(IF, WHILE, FOR, MATCH, TRY, NEW, THROW, GIVEN, VAL, THIS)

final val softModifierNames = Set(nme.inline, nme.opaque, nme.open, nme.transparent, nme.infix)
}
63 changes: 31 additions & 32 deletions docs/docs/reference/changed-features/match-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,41 @@ title: Match Expressions
The syntactical precedence of match expressions has been changed.
`match` is still a keyword, but it is used like an alphabetical operator. This has several consequences:

1. `match` expressions can be chained:

```scala
xs match {
case Nil => "empty"
case x :: xs1 => "nonempty"
} match {
case "empty" => 0
case "nonempty" => 1
}
```

(or, dropping the optional braces)

```scala
xs match
case Nil => "empty"
case x :: xs1 => "nonempty"
match
case "empty" => 0
case "nonempty" => 1
```

2. `match` may follow a period:
1. `match` expressions can be chained:

```scala
if xs.match
case Nil => false
case _ => true
then "nonempty"
else "empty"
xs match {
case Nil => "empty"
case x :: xs1 => "nonempty"
} match {
case "empty" => 0
case "nonempty" => 1
}
```

3. The scrutinee of a match expression must be an `InfixExpr`. Previously the scrutinee could be
followed by a type ascription `: T`, but this is no longer supported. So `x : T match { ... }`
now has to be written `(x: T) match { ... }`.
(or, dropping the optional braces)

```scala
xs match
case Nil => "empty"
case x :: xs1 => "nonempty"
match
case "empty" => 0
case "nonempty" => 1
```

2. `match` may follow a period:

```scala
if xs.match
case Nil => false
case _ => true
then "nonempty"
else "empty"
```

3. The scrutinee of a match expression must be an `InfixExpr`. Previously the scrutinee could be followed by a type ascription `: T`, but this is no longer supported. So `x : T match { ... }` now has to be
written `(x: T) match { ... }`.

## Syntax

Expand Down
4 changes: 2 additions & 2 deletions docs/docs/reference/other-new-features/indentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ There are two rules:
- after one of the following tokens:

```
= => <- catch do else finally for
if match return then try while yield
= => ?=> <- catch do else finally for
if match return then throw try while yield
```

If an `<indent>` is inserted, the indentation width of the token on the next line
Expand Down
42 changes: 42 additions & 0 deletions docs/docs/reference/other-new-features/matchable.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,45 @@ class Object extends Any, Matchable

`Matchable` is currently a marker trait without any methods. Over time
we might migrate methods `getClass` and `isInstanceOf` to it, since these are closely related to pattern-matching.

### `Matchable` and Universal Equality

Methods that pattern-match on selectors of type `Any` will need a cast once the
Matchable warning is turned on. The most common such method is the universal
`equals` method. It will have to be written as in the following example:

```scala
class C(val x: String):

override def equals(that: Any): Boolean =
that.asInstanceOf[Matchable] match
case that: C => this.x == that.x
case _ => false
```
The cast of `that` to `Matchable` serves as an indication that universal equality
is unsafe in the presence of abstract types and opaque types since it cannot properly distinguish the meaning of a type from its representation. The cast
is guaranteed to succeed at run-time since `Any` and `Matchable` both erase to
`Object`.

For instance, consider the definitions
```scala
opaque type Meter = Double
def Meter(x: Double) = x

opaque type Second = Double
def Second(x: Double) = x
```
Here, universal `equals` will return true for
```scala
Meter(10).equals(Second(10))
```
even though this is clearly false mathematically. With [multiversal equality](../contextual/multiversal-equality.html) one can mitigate that problem somewhat by turning
```scala
Meter(10) == Second(10)
```
into a type error.





12 changes: 10 additions & 2 deletions tests/pos/indent.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ object Test:
y * y
}

val gg = (x: Int) ?=>
val y = x + 1
y

xs.map {
x =>
val y = x * x
Expand Down Expand Up @@ -80,11 +84,15 @@ object Test:

class Test2:
self =>
def foo = 1
def foo(x: Int) =
if x < 0 then throw
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to allow this? I somehow parse this as:

if x < 0 then { throw();
  val ex = ...
}

Copy link
Contributor Author

@odersky odersky Dec 24, 2020

Choose a reason for hiding this comment

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

For consistency I think it is better to allow it. The given expression abuses indentation rules for sure. It should be formatted like this:

if x < 0 then
   throw
      val ex = ...
      ex

val ex = new AssertionError()
ex
x

val x =
new Test2 {
override def foo = 2
override def foo(x: Int) = 2
}
end x
end Test2
Expand Down