Update -Yindent-colons functionality #11752

odersky · 2021-03-15T16:18:19Z

Move functionality under -Yindent-colons closer to what we discussed in the contributors thread

Replace -Yindent-colons with experimental.fewerBraces language feature
Accept : at EOL as a normal : if it cannot start an argument
Use normal indentation rules after return; no : required or allowed
Allow closure arguments to be passed without enclosing braces

NOTE: The changes to indentation after return are in standard Scala 3. The other implemented functionality is under an language import import experimental.fewerBraces. It's not part of Scala 3.0 and might still change in the future.

I also attempted a change to make indentation significant after symbolic operators. But I had to abandon that part, since it changes semantics. Consider

  val x = a -
     b + c

If we made indentation significant after -, the expression would be parsed as a - (b + c) instead of (a - b) + c. So we can't do that.

Systamtically accept a `:` as COLON when it cannot be confused with a `:` at EOL.

Treat instead indentation as significant. I.e. return 2 // one statement return println(2) // two statements

Replace -Yindent-colons with language import import language.experimental.fewerBraces

This allows a useful pattern for expressing curried function arguments with `:`. E.g. files.get(fileName).fold: "not found" : f => f.readLine() Without the change here, the second `:` would have to be on the same column as `files`.

Two reasons: 1. It would complicate the syntax 2. It looks weird anyway

Some users might write things like `then:` by accident. In this case we should report an error, skip the `:`, and continue. No skip ahead to an anchor token is wanted.

odersky · 2021-03-19T13:38:55Z

@tgodzik Do you want to review this? That would be a good start to adapt Metals accordingly. We need this merged by Monday.

tgodzik

Thanks for letting me know! We do not support the experimental flag in scalameta yet. We want to make most tooling work for the basic optional braces first. Which means, we are focusing on scalafmt currently.

I do have my doubts about this syntax, but it might be better to leave up to the community to give some feedback on this.

tgodzik · 2021-03-19T14:06:31Z

compiler/src/dotty/tools/dotc/parsing/Tokens.scala

@@ -273,7 +273,7 @@ object Tokens extends TokensCommon {
  final val closingRegionTokens = BitSet(RBRACE, RPAREN, RBRACKET, CASE) | statCtdTokens

  final val canStartIndentTokens: BitSet =
-    statCtdTokens | BitSet(COLONEOL, WITH, EQUALS, ARROW, CTXARROW, LARROW, WHILE, TRY, FOR, IF, THROW)
+    statCtdTokens | BitSet(COLONEOL, WITH, EQUALS, ARROW, CTXARROW, LARROW, WHILE, TRY, FOR, IF, THROW, RETURN)


That's a global change, not only for the experimental flag? We should update it in Scalameta then, but should not be problematic.

Yes, it's a global change

scalameta/scalameta#2264

tgodzik · 2021-03-19T14:10:00Z

docs/docs/reference/other-new-features/indentation-experimental.md

+      val fileNames = files.values
+      s"""no file named $fileName found among
+         |${values.mkString(\n)}""".stripMargin
+   :


Are we able to handle this another way? It seems a bit wrong? Kind of like someone left the colon by mistake.

Maybe something like a soft keyword and ?

I think curried functions with complex arguments are pretty much an anti-pattern anyway, but others like their Option.folds. The current possibility falls out from the rules we have. Before this PR there was a special added rule that disallowed the : at start of line. But that would leave no way to handle curried functions except with braces. So I think it's probably better to have simpler, more orthogonal rules and let users decide whether they like this pattern or whether they want to keep using braces after all.

It feels like it might look better if there will be an additional per-parameter indentation.
Smth like:

val firstLine = files.get(filename).fold: {smth here} => val filenames = files.values s"""no file named $fileName found among |${values.mkString(\n)}""".stripMargin f => val lines = f.iterator.map(_.readline) lines.mkString("\n")

That does not work. This would group both lambdas in a sequence as a single parameter block.

Yep, I missed that : is about different parameter sections.

Should this work if : have the same indentation level as the first parameter?

val firstLine = files.get(filename).fold: val filenames = files.values s"""no file named $fileName found among |${values.mkString(\n)}""".stripMargin : f => val lines = f.iterator.map(_.readline) lines.mkString("\n")

No, then it does not work. Everything at the same indentation level is one argument

tgodzik · 2021-03-19T14:17:02Z

docs/docs/reference/other-new-features/indentation-experimental.md

+
+Similar to what is done for classes and objects, a `:` that follows a function reference at the end of a line means braces can be omitted for function arguments. Example:
+```scala
+times(10):


This looks rather ok, but honestly I am not sure if it helps with the readability. But I guess if we are going fully with optional braces, we should allow this.

tests/run/returns.scala

tgodzik · 2021-03-19T14:39:40Z

tests/run/returns.scala

+  else
+    2
+
+def bar(): Unit =


Is this equivalent to:

def bar(): Unit = return assert(false)

?

No; assert(false) is dead code in that case.

Previously, return could not start an indentation, but starting with scala/scala3#11752 it can. This fixes scalameta to parse accordingly.

This reverts commit 0bd9739. Change syntax and tests to allow `:` after operator. It turns out that the intent project of the CB already uses them in a way that looks quite reasonable.

odersky · 2021-03-24T08:32:21Z

@tgodzik Is this ready to merge?

tgodzik

Looks good on my side!

Previously, return could not start an indentation, but starting with scala/scala3#11752 it can. This fixes scalameta to parse accordingly. Additionally, this change also created some issues with empty `return` statements in pattern matches. We would incorrectly think that the next statement is indented, because we never saved case block indentation. I added a new CaseRegion in this PR, so that we can remember the indentation. This is only added in `match` with braces as indented match clauses work correctly here.

hejfelix · 2021-04-07T06:26:12Z

I just want to mention that it's pretty jarring not to have this feature currently. See e.g. munit test syntax -- you are forced to use braces for every test:

test("mytest") {
  assert(true)
}

odersky · 2021-04-07T10:36:08Z

I agree. It's the maximum we could achieve against community pushback and general skepticism. If you want this, you can help by demanding the functionality and defending it vigorously in contributors.

…

On Wed, Apr 7, 2021 at 8:26 AM Felix Palludan Hargreaves < ***@***.***> wrote: I just want to mention that it's pretty jarring not to have this feature currently. See e.g. munit test syntax -- you are *forced* to use braces for every test: test("mytest") { assert(true) } — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

-- Prof. Martin Odersky LAMP/IC, EPFL

hmf · 2021-04-12T09:27:38Z

I have another use case in which makes this useful - example code were we want to shadow variables with the same name (reinventing names is tedious). Code for students using the DJL Java library, braces here just seems excessive.

    val manager = NDManager.newBaseManager()
    val x = manager.create(Array(1, 2, 3))
    println(s"x.getDevice() = ${x.getDevice()}")

    // 5.5.2.1. Storage on the GPU: https://d2l.djl.ai/chapter_deep-learning-computation/use-gpu.html#storage-on-the-gpu
    locally: 
      val x = manager.ones(new Shape(2, 3), DataType.FLOAT32, tryGpu(0))
      println(s"x = $x")
      // Second GPU?
      val y = manager.ones(new Shape(2, 3), DataType.FLOAT32, tryGpu(0))
      println(s"y = $y")

      // 5.5.2.2. Copying: https://d2l.djl.ai/chapter_deep-learning-computation/use-gpu.html#copying
      val z = x.toDevice(tryGpu(1), true)
      println(s"Gpu(0) x = $x")
      println(s"Gpu(1) z = $z")    

      // Now that the data is on the same GPU (both z and y are), we can add them up.
      y.add(z)

If you want this, you can help by demanding the
functionality and defending it vigorously in contributors.

I take it that would be in this link? Seems to be closed though.

https://contributors.scala-lang.org/t/feedback-sought-optional-braces/4702/587

But now I cannot use this in RC2:

 import language.experimental.fewerBraces

because of:

#11920

or am I wrong?

TIA

odersky added 4 commits March 18, 2021 11:10

Accept more : at EOLs under -Yindent-colon

1d07c11

Systamtically accept a `:` as COLON when it cannot be confused with a `:` at EOL.

Disallow : after return

20d2e2a

Treat instead indentation as significant. I.e. return 2 // one statement return println(2) // two statements

Replace -Yindent-colons with language import

cd01d25

Replace -Yindent-colons with language import import language.experimental.fewerBraces

Drop -Yindent-colons in CB

e80a192

odersky force-pushed the fix-indent-colon branch from 08853d4 to e80a192 Compare March 18, 2021 11:12

odersky added 4 commits March 18, 2021 13:12

Use isArrow utility method in Parser

53928a9

Generalize TokenData#isOperator

2093b93

Add closure arguments, first version

dbfde1c

Enable closure arguments only under fewerBracesEnabled

a57134e

odersky linked an issue Mar 19, 2021 that may be closed by this pull request

indentation region after return keyword is not recognized #11802

Closed

odersky mentioned this pull request Mar 19, 2021

indentation region after return keyword is not recognized #11802

Closed

odersky added 4 commits March 19, 2021 14:27

Allow to retract to new column if next token is :

db19cfd

This allows a useful pattern for expressing curried function arguments with `:`. E.g. files.get(fileName).fold: "not found" : f => f.readLine() Without the change here, the second `:` would have to be on the same column as `files`.

Disallow : after infix operators

0bd9739

Two reasons: 1. It would complicate the syntax 2. It looks weird anyway

Improve error recovery after illegal :

e5fd587

Some users might write things like `then:` by accident. In this case we should report an error, skip the `:`, and continue. No skip ahead to an anchor token is wanted.

More tests

0c57527

odersky marked this pull request as ready for review March 19, 2021 13:37

odersky requested a review from tgodzik March 19, 2021 13:37

odersky assigned tgodzik Mar 19, 2021

tgodzik reviewed Mar 19, 2021

View reviewed changes

tgodzik mentioned this pull request Mar 19, 2021

[Scala 3] Allow return statement to contain indented block scalameta/scalameta#2264

Merged

odersky added 2 commits March 19, 2021 16:26

Augment returns test

8cd6337

Revert "Disallow : after infix operators"

9fd873c

This reverts commit 0bd9739. Change syntax and tests to allow `:` after operator. It turns out that the intent project of the CB already uses them in a way that looks quite reasonable.

odersky added this to the 3.0.0-RC2 milestone Mar 22, 2021

tgodzik approved these changes Mar 24, 2021

View reviewed changes

odersky merged commit 7580ee9 into scala:master Mar 24, 2021

odersky deleted the fix-indent-colon branch March 24, 2021 16:38

Kordyjan modified the milestones: 3.0.0-RC2, 3.0.0 Aug 2, 2023

Update -Yindent-colons functionality #11752

Update -Yindent-colons functionality #11752

Uh oh!

Conversation

odersky commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgodzik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

odersky commented Mar 24, 2021

Uh oh!

tgodzik left a comment

Choose a reason for hiding this comment

Uh oh!

hejfelix commented Apr 7, 2021

Uh oh!

odersky commented Apr 7, 2021 via email

Uh oh!

hmf commented Apr 12, 2021

Uh oh!

Uh oh!

odersky commented Mar 15, 2021 •

edited

Loading

odersky commented Mar 19, 2021 •

edited

Loading