Feedback sought: Optional Braces

jducoeur · December 10, 2020, 8:15pm

Currently slammed at work (so I sadly don’t have time to read the entire thread), but having been one of the more vocal people on this topic I should chime in:

I’ve largely made my peace with the existence of this feature, and having played with it a little, I like it in the small. My problem with it is mainly that in the large, I find code much harder to read. For a few small functions, it’s great; for a complex class with lots of functions (and especially as things get nested), I just plain find it challenging to read the code. I’ve found many of the examples that have been recently posted more difficult to skim and grok than they should be. So I think I’m unlikely to convert over to this syntax entirely, although I suspect I would use it a moderate amount.

More seriously: working at a company with hundreds of Scala engineers and hundreds of Scala repos, many of them legacy code that isn’t likely to upgrade quickly, there is a serious risk of style skew and confusion. We take code style very seriously – while we don’t enforce a single style religiously, we spent literally six months arguing about the pros and cons of 80 vs 100 vs 120 character line widths last year. Style matters when you have large numbers of people who need to be able to easily play in each others’ code.

(Not to mention the training and social cost. We want to minimize the effort of Scala 3 adoption among our engineers, and I think this is likely to be a hurdle. I should note that, when we talk about it at work, some folks express more intense dislike than mine – I don’t seem to be an outlier, far as I can tell.)

It’s not easy to see how a dramatic style shift like this works in that environment: I suspect that, at most, it would be a matter of gradual and partial adoption over a span of years, and I’m not sure that it’s ever going to become the dominant style at work.

Which brings me to my big problem here: the degree to which the Dotty team has gone whole-hog on this. It often seems to be treated as the new style, not an alternative. That provokes a lot of gut-wrenching anxiety, frankly: it feels like the current stage is simply a waypoint towards deprecating braces entirely. I’m not sure whether that’s the intent, but it’s how it has increasingly been coming off to me, and that I think would be a Huge Problem.

So fundamentally, I’m okay with the Optional Braces feature provided I can ignore it. So long as it is truly optional, and stays optional, I don’t have a big problem with it. But it’s come to feel like such a juggernaut, with so many people enthusing that This Is The One True New Way, that I can’t say I’m sanguine there. The lack of a sense of the long-term intent is unsettling…

mbloms · December 10, 2020, 8:43pm

I understand this argument, though I don’t actually think this is a problem in practice. Having worked quite a bit with Haskell, I’m well aware of the big cost of having weird symbols compared to the cost of just using simple words with meaning. For example: which is clearer: >>= or flatMap?

I would welcome any alternative to where, but my personal opinion is that none of the suggested alternatives: $, #, @, .., are better. They all require an explanation.

This is why I suggest let…in which is far clearer when you have a return value that you want to emphasis.

These mean different things. = is unchanged from the current syntax. All I did to givens was to change with to where and : to with.

The motivation is that where consistently opens a template body. While with opens a refinement type body.

rjolly · December 10, 2020, 8:53pm

github.com/lampepfl/dotty

Givens without `as`

lampepfl:master ← dotty-staging:change-given-syntax-3

opened 06:20PM - 28 Nov 20 UTC

odersky

+690 -596

Changes in a nutshell: ```scala given intOrd: Ordering[Int] with ... giv…en listOrd[T: Ordering]: Ordering[List[T]] with ... given Ordering[Int] with ... given [T: Ordering]: Ordering[List[T]] with ... given global: ExecutionContext = ForkJoinContext() given Context = ctx ``` instead of the current syntax: ```scala given intOrd as Ordering[Int]: ... given listOrd[T: Ordering] as Ordering[List[T]]: ... given Ordering[Int]: ... given [T: Ordering] as Ordering[List[T]]: ... given global as ExecutionContext = ForkJoinContext() given Context = ctx ``` ## Syntax Here is the syntax for given instances: ``` TmplDef ::= ... | ‘given’ GivenDef GivenDef ::= [GivenSig] StructuralInstance | [GivenSig] Type ‘=’ Expr | [GivenSig] Type GivenSig ::= [id] [DefTypeParamClause] {UsingParamClause} ‘:’ StructuralInstance ::= ConstrApp {‘with’ ConstrApp} ‘with’ TemplateBody ``` A given instance starts with the reserved word `given` and an optional _signature_. The signature defines a name and/or parameters for the instance. It is followed by `:`. There are three kinds of given instances: - A _structural instance_ contains one or more types or constructor applications, followed by `with` and a template body that contains member definitions of the instance. - An _alias instance_ contains a type, followed by `=` and a right hand side expression. - An _abstract instance_ contains just the type, which is not followed by anything. ## How did we get here? The previous syntax for givens provided alias givens and structural givens like this. ```scala given x as C = e given x as C {} ``` It's the only instance in the grammar where a construct can be followed by either an `=` and an alias or a template in braces. Everywhere else we distinguish between the two. The main reason for this is what happens if there is neither a `=` nor a `{`. I.e. - `val x: T` is an _abstract_ val, whereas - `object x extends T` is an _empty_ object That's why I felt that we need a keyword after the name, to emphasize that we get a concrete `given` with an empty body if followed by nothing. But what if we turn around that decision? I.e. what if we take an empty continuation as an abstract given? Several people have presented use cases for abstract givens. While it's true that they can be encoded, the encodings do lead to some duplication, so at first sight it seems like a welcome generalization to support abstract givens. If we do switch that convention, then we do not need a keyword as connective anymore. Arguably `:` is the right choice then. But if we do that there should be something heavier than just `{}` to distinguish between an abstract `given x: T` and a concrete given `given x: T{}`, in particular since `T{}` can be read as a refinement type. Everywhere else the choice between abstract and concrete is made clear by the absence or presence of `=`. We can't use `=` since we expect a block after an `=`, not a template body containing definitions. So we need something else. In this PR, the connective starting a template body is `with`. ## Ramifications and Speculations This has another interesting consequence. Taken by itself, can we give a meaning to `T with { defs }`? In fact, this does make sense as an alternative syntax for an anonymous class. If we go down that path, we can truly get rid of all vestiges of `new` in the syntax (over time, not for 3.0). Taking this further, we can now see the following (approximate) equivalence: ```scala given x: C with { ... } ~~~ given x: C = C with { ... } ~~~ given x: C = new C {...} ``` So, `: C with {...}` is approximately `: C = C with {...}`, and it avoids the repetition. In fact `: C with {}` is more useful than `: C = C with {...}` since it keeps any refinements in `{...}` in the type. So it's more like a RHS `= new {...}` and an inferred result type. In fact it's even better than that, since an anonymous class `new { ... }` is subject to avoidance, but `: C with { .. }` creates the class alongside the given instead of in its right hand side. This means that members freshly introduced in `{...}` are visible in the result of the given. To see the difference, consider this code: ```scala class C given c: C with def foo = 1 given d: C = new C { def foo = 1 } def test = c.foo // OK d.foo // error: `foo` is not a member of `d`. ``` Now, one intriguing step further is whether we want to allow the same convention for normal defs. I.e ```scala def foo(x: Int): T with {...} ``` as a slightly more powerful alternative to ```scala def foo(x: Int) = T with {...} ``` or, using `new` ```scala def foo(x: Int) = new T {...} ``` with the same expansion as for the `given`. This would give us parameterized objects, or functors in the SML sense, without having to define a class. It also gives a nice way to avoid duplication between return type and RHS while stile having an explicitly defined return type. ## Summary The new design leads to a strong similarity between givens and other definitions. Essentially - `given` expands to `lazy val` or `def` (we leave it to the system to decide), and the name is optional - the rest is exactly as for vals and defs This makes the language more regular. Ity also makes it very easy to switch from explicit definitions to givens and back. Other benefits: - Anonymous classes can be written without `new` - Abstract givens are supported - A powerful way to get parameterized objects without having to go through classes ## Timeline The change to the given syntax, including support for abstract givens, would have to be done before 3.0 release. The rest could come later.

odersky · December 10, 2020, 9:06pm

I can guarantee that braces will never go away. Even if the whole Scala ecosystem converted to indentation we still want to keep them around as a way to serialize programs, similar to their role in Haskell.

What could change at some point in the future is control syntax. There are no concrete plans to deprecate old-style syntax with (...) around conditions, but if the new syntax is adopted pervasively there might be a time where we want to migrate to it exclusively.

rjolly · December 10, 2020, 9:23pm

My suggestion:

given righteousDude: Record with Dude = new
  val name: String = "Frank"
  val age: Int = "57"
  val righteous: Boolean = true

new becomes yet another keyword that can open an indented block. This is less powerful than what is proposed in the “Given without as” PR:

But I think it is going a bit too far, we don’t need freshly introduced members, we can define a class for it.

curoli · December 10, 2020, 9:22pm

Thanks, @odersky I’m very relieved. I was already looking at Kotlin tutorials.

odersky · December 10, 2020, 9:38pm

What is the reason for this rule? (emphasis mine)

Indentation tokens are only inserted in regions where newline statement separators are also inferred: at the toplevel, inside braces {...} , but not inside parentheses (...) , patterns or types.

Just being conservative. We never inferred newlines inside parens. We might want to try to change that at some point, which would need a careful evaluation of how much code would break. But it’s not needed for the current design, so we did not undertake it.

japgolly · December 10, 2020, 10:00pm

I guess it’s time to get counter-opinions on the .. proposal. I’m sorry to say that I hate it, and surprisingly passionately at that. I really dislike the way it looks, not to mention that it’s taking up three chars now instead of one.

I really think the : is perfect. I don’t understand the arguments that code becomes unreadable or ambiguous. Why? We all know that you need a = between a declaration and a definition. No one is writing code like [def nonEmpty: Boolean !isEmpty] and spending minutes wondering why it doesn’t compile. Similarly I can’t understand how these can be conflated

def nonEmpty:
  Boolean // silly but ok

def nonEmpty: Boolean =
  !isEmpty

My mental approach is to look for the equal sign. Before reading this thread I just kind of assumed that everyone is like that. Regardless, I can’t imagine that I’m some weird outlier in this regard so yeah, with this post I make our voice heard, for we are many! We are the downtrodd- (ok ok ok…)

adjtr · December 10, 2020, 10:14pm

See jdegoes post. Could we get your views on this @odersky? It feels by far to be the cleanest and simplest approach.

Ichoran · December 10, 2020, 10:30pm

I decided to grab some random bits of code to stress-test indentation.

Random stress test one: class with long parameter list:

class TierTwo(
  now: Instant,
  one: Path, two: Path, net: Path,
  parallelism: Int, stopBy: Instant,
  quiet: Boolean = false,
  policies: Tiering.BackupPolicies = Tiering.BackupPolicies.default,
  parameters: RunMwt.Parameters = RunMwt.Parameters.default
) { outer =>
  import Tiering._
  ...
  def run() = {
    ...
    (tn, tw, tr, bn, bw, br, an, aw, ar)
  }
}

With a colon:

class TierTwo(
  now: Instant,
  one: Path, two: Path, net: Path,
  parallelism: Int, stopBy: Instant,
  quiet: Boolean = false,
  policies: Tiering.BackupPolicies = Tiering.BackupPolicies.default,
  parameters: RunMwt.Parameters = RunMwt.Parameters.default
):
  val outer = this
  import Tiering._
  ...
  def run() =
    ...
    (tn, tw, tr, bn, bw, br, an, aw, ar)

Verdict: ): says it all. Sad face, pointing backwards. It’s a step back to readability to me just because it’s so not-there.

class TierTwo(
  now: Instant,
  one: Path, two: Path, net: Path,
  parallelism: Int, stopBy: Instant,
  quiet: Boolean = false,
  policies: Tiering.BackupPolicies = Tiering.BackupPolicies.default,
  parameters: RunMwt.Parameters = RunMwt.Parameters.default
) with outer =>
  import Tiering._
  ...
  def run() =
    ...
    (tn, tw, tr, bn, bw, br, an, aw, ar)

Verdict: slightly better, but still not very visually clear. Would probably use braces regardless. Prefer with over :. (Not shown: .. also gets lost.

Random stress test two: complex method with lamba(s):

    val (contested, uncontested) = unsaved.map{
      case (f, t, _, Some(sf)) => No((f, t, sf))
      case (f, t, c, _) =>
        val g = targetFrom(c, remote) \: (f % "zip").getName
        if (safe{ g.exists }.yesOr(_ => false)) No((f, t, g))
        else {
          val ga = targetFrom(c, remote) \: (f % "zip.atomic").getName
          if (safe{ ga.exists }.yesOr(_ => false)) No((f, t, ga))
          else Yes((f, t, c))
        }
    }.unmix

With colons:

    val (contested, uncontested) = unsaved
      .map:
        case (f, t, _, Some(sf)) => No((f, t, sf))
        case (f, t, c, _) =>
          val g = targetFrom(c, remote) \: (f % "zip").getName
          if (safe{ g.exists }.yesOr(_ => false)) No((f, t, g))
          else
            val ga = targetFrom(c, remote) \: (f % "zip.atomic").getName
            if (safe{ ga.exists }.yesOr(_ => false)) No((f, t, ga))
            else Yes((f, t, c))
      .unmix

Verdict: Colon works fine.

With dots:

    val (contested, uncontested) = unsaved
      .map ..
        case (f, t, _, Some(sf)) => No((f, t, sf))
        case (f, t, c, _) =>
          val g = targetFrom(c, remote) \: (f % "zip").getName
          if (safe{ g.exists }.yesOr(_ => false)) No((f, t, g))
          else
            val ga = targetFrom(c, remote) \: (f % "zip.atomic").getName
            if (safe{ ga.exists }.yesOr(_ => false)) No((f, t, ga))
            else Yes((f, t, c))
      .unmix

Equally good.

Not shown: with is clunky.

Stress test three: various small objects and classes.

    class Zip(home: Path) extends Three(home) with Archived { zip => 
      type S = Record.Three.Zip[this.type]
      val creator = new InPlateSlot[S, zip.type] {
        val owner = zip
        protected def uncheckedCreate(home: Path, sd: SpannerDir): S =
          new Record.Three.Zip[zip.type](zip, home, sd)
        protected val decider =
          (s: String) => Yes(s == "zip")
      }
    }
    object Zip {
      def apply(home: Path) = new Zip(home)

      def old(home: Path) = new OldZip(home)
      class OldZip(home: Path) extends Zip(home) {
        override def versioning = Versioning.OhOneA
      }
    }

Colons:

    class Zip(home: Path) extends Three(home) with Archived:
      private[this] def zip = this  // Is there a better way?
      type S = Record.Three.Zip[this.type]
      val creator = new InPlateSlot[S, zip.type]:
        val owner = zip
        protected def uncheckedCreate(home: Path, sd: SpannerDir): S =
          new Record.Three.Zip[zip.type](zip, home, sd)
        protected val decider =
          (s: String) => Yes(s == "zip")
    object Zip:
      def apply(home: Path) = new Zip(home)

      def old(home: Path) = new OldZip(home)
      class OldZip(home: Path) extends Zip(home):
        override def versioning = Versioning.OhOneA

Nice and compact, but the :'s are really important and tend to vanish into the clutter. Verdict: an improvement over the status quo.

With with:

    class Zip(home: Path) extends Three(home) with Archived with
      private[this] def zip = this  // Is there a better way?
      type S = Record.Three.Zip[this.type]
      val creator = new InPlateSlot[S, zip.type] with
        val owner = zip
        protected def uncheckedCreate(home: Path, sd: SpannerDir): S =
          new Record.Three.Zip[zip.type](zip, home, sd)
        protected val decider =
          (s: String) => Yes(s == "zip")
    object Zip with
      def apply(home: Path) = new Zip(home)

      def old(home: Path) = new OldZip(home)
      class OldZip(home: Path) extends Zip(home) with
        override def versioning = Versioning.OhOneA

Considerably easier to spot object boundaries. Equally compact to :.

Verdict: an even bigger improvement over the status quo, and an improvement over :.

Not shown: ... Slightly easier to spot than : but suffers the same problems.

Stress test four: chained/fluent style

    broke.foreach{ case (_, msg) => 
      alertables.
        map(a => (a, a alert msg)).
        collect{ case (a, No(e)) =>
          s"Failed to send alert $a\n=========\n$e\n==========\n\n"
        }.
        mkString.
        tap(x => if (x.nonEmpty) println(x))
    }

With colons:

  broke.foreach:
   case (_, msg) =>
      alertables
      .map:
        a => (a, a alert msg)
      .collect:
        case (a, No(e)) =>
          s"Failed to send alert $a\n=========\n$e\n==========\n\n"
      .mkString
      .tap:
        x => if (x.nonEmpty) println(x)

Verdict: requirements for : render this no better and possibly worse than the brace style for me (since : must end a line). Wouldn’t use. Might use with mixed style (braces in-line), but mixed style clashes.

Dots:

  broke.foreach ..
    case (_, msg) => 
      alertables
      .map .. a =>
        (a, a alert msg)
      .collect .. case (a, No(e)) =>
        s"Failed to send alert $a\n=========\n$e\n==========\n\n"
      .mkString
      .tap .. x =>
        if (x.nonEmpty) println(x)

Lovely! A big improvement over both status quo and :. The method .. varname => pattern makes what is happening really stand out, but without extra space.

Overall–I’m now less excited by the feature than I thought I would be after the discussion above. (I originally wasn’t very excited, but the discussion had gotten me more excited.)

I don’t think dots work for template definitions. with is perfect.

However, I am quite enthusiastic about .. instead of -Yindent-colons. As far as I can tell it solves all problems associated with the syntax. It also makes sense that template definitions are not the same as arguments.

In particular, I think .. works so well because it is a different symbol in a context where : is really expected to be a type ascription, and it enables inline use as well as block-starting use. So you can either start your block with .. or with => depending on what makes sense for the formatting.

If it were up to me, I’d

Remove -Yindent-colons and have -Yblock-dots instead, or simply just include it by default. I think it works well enough to not need to go behind a Y-flag.
Allow with to open template definitions, including with braces.
Allow : as an alternative to with for opening template definitions. with: would be okay too.

This would preserve existing tutorial materials, have a highly regular syntax for people who like regularity, and allow usage to determine the amount of optionality people accept. Furthermore, it would make braceless style first-class as you could write lambdas bracelessly.

japgolly · December 10, 2020, 11:48pm

lihaoyi · December 11, 2020, 1:03am

I’d like to chip in to say that one thing I see everyone getting wrong here is that not needing closing curlies does not mean you delete all the lines where the closing curlies previously were! This is not an encouraged style in any whitespace-delimited programming language, and for good reason. Consider Python’s PEP8 guidance on blank lines:

Surround top-level function and class definitions with two blank lines.

Method definitions inside a class are surrounded by a single blank line.

Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

Use blank lines in functions, sparingly, to indicate logical sections.

For example, @Ichoran’s code snippets above read a lot better if spaced out, in a PEP8-inspired style:

    class Zip(home: Path) extends Three(home) with Archived:
      private[this] def zip = this  // Is there a better way?
      type S = Record.Three.Zip[this.type]

      val creator = new InPlateSlot[S, zip.type]:
        val owner = zip

        protected def uncheckedCreate(home: Path, sd: SpannerDir): S =
          new Record.Three.Zip[zip.type](zip, home, sd)

        protected val decider =
          (s: String) => Yes(s == "zip")


    object Zip:
      def apply(home: Path) = new Zip(home)
      def old(home: Path) = new OldZip(home)

      class OldZip(home: Path) extends Zip(home):
        override def versioning = Versioning.OhOneA

Honestly I think a lot of @odersky’s code would also look better if appropriately spaced out. e.g. this section of code looks pretty impenetrable to me now:

        case tree: Select =>
          val qual = tree.qualifier
          val qualSpan = qual.span
          val sym = tree.symbol.adjustIfCtorTyparam
          registerUseGuarded(qual.symbol.ifExists, sym, selectSpan(tree), tree.source)
          if qualSpan.exists && qualSpan.hasLength then
            traverse(qual)
        case tree: Import =>
          if tree.span.exists && tree.span.hasLength then
            for sel <- tree.selectors do
              val imported = sel.imported.name
              if imported != nme.WILDCARD then
                for alt <- tree.expr.tpe.member(imported).alternatives do
                  registerUseGuarded(None, alt.symbol, sel.imported.span, tree.source)
                  if (alt.symbol.companionClass.exists)
                    registerUseGuarded(None, alt.symbol.companionClass, sel.imported.span, tree.source)
            traverseChildren(tree)

but with some PEP8-inspired spacing to let the code breathe it looks a lot better:

        case tree: Select =>
          val qual = tree.qualifier
          val qualSpan = qual.span
          val sym = tree.symbol.adjustIfCtorTyparam

          registerUseGuarded(qual.symbol.ifExists, sym, selectSpan(tree), tree.source)

          if qualSpan.exists && qualSpan.hasLength then
            traverse(qual)

        case tree: Import =>
          if tree.span.exists && tree.span.hasLength then
            for sel <- tree.selectors do
              val imported = sel.imported.name

              if imported != nme.WILDCARD then
                for alt <- tree.expr.tpe.member(imported).alternatives do
                  registerUseGuarded(None, alt.symbol, sel.imported.span, tree.source)

                  if (alt.symbol.companionClass.exists)
                    registerUseGuarded(None, alt.symbol.companionClass, sel.imported.span, tree.source)

            traverseChildren(tree)

I’ll mention again that four-space indents really make a big different for indent-delimited blocks, and would make these examples much much clearer and easier to skim. But even with two-space indents, blank lines are important, and the current way I see everyone taking advantage of indent-deleted code to delete all vertical whitespace in their programs is very much no good.

Ichoran · December 11, 2020, 1:19am

You’re probably right.

However, since being able to read more per screen is about the only advantage I see in the syntax, this makes me even less enthusiastic about it. Just add the braces back in, in the same amount of space, and ta-da, clearer-to-read, more safely-parsed code.

AMatveev · December 11, 2020, 6:56am

When I had started to work with python I was really surprised that idea strongly required to paste spaces(empty lines). It is so important that the lack of spaces is highlighted by worrings. So one of the main advantage of syntax can lead to worse reading. The situation will getting more worse for large codebase with high level of block’s hierarchy. OK, it is very nice for science, especially when they need to print code.

But why is such style planning to become default for all community? Why is there such hurry?

odersky · December 11, 2020, 8:17am

I think I found a solution for the problem what combinator to use for terms such as map or exists: Nothing at all! I.e.

xs.map x =>
  x + 1

xs.foreach x =>
  println(x)

(xs, ys).lazyZip (x, y) =>
  x + y

Grammatically, this does not pose any new parsing problems. We already need to be prepared to parse either an expression or a parameter list when we parse top-level lambdas. And visually these look alright as long as we restrict the right operand to a multi-line lambda, with the => ending a line. I would be more sceptical about allowing

xs.foreach x => println(x)

I believe that would lend itself to obscure code.

EDIT: Or maybe we can allow single line lambdas but nothing should follow them. I.e. this would be atrocious:

xs.map x => (x + 1).filter x => (x > 0)

mbloms · December 11, 2020, 8:34am

I think this makes sense. I was thinking of proposing it myself. Given that lambdas are allowed without braces as a single expression, it’s logical.

For these examples however I still think that having do around as an alternative would make sense:

xs.map do
  x => x + 1

xs.foreach do
  x => println(x)

(xs, ys).lazyZip do
  (x, y) => x + y

odersky · December 11, 2020, 8:34am

I agree. The difference is you are back in control of your whitespace. You don’t need to add multiple blank lines for a cascade of closing braces; one is good enough. You also can align else with if without having to spend an extra line for a closing brace in front of it. And so on.

mbloms · December 11, 2020, 8:39am

@odersky it’s been proposed several times in this thread that either with or where is introduced as an alternative to : before template bodies. Is this something you’re at all open to, or is it just an unacceptable compromise?

bmeesters · December 11, 2020, 8:39am

I think it would be a really hard sell to call Scala an FP/OOP blend and disallow single line lambdas. I am 100% sure that we would then not use ‘braceless’ style for higher-order functions in my company.

mbloms · December 11, 2020, 8:42am

xs.foreach
  x => println(x)

Might actually be more readable in that case. It too makes sense that it’s allowed if the lambda is a single expression.