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

Drop do while #6994

Merged
merged 7 commits into from Aug 6, 2019

Conversation

@odersky
Copy link
Contributor

commented Aug 6, 2019

Drop

 do <body> while <cond>

in favor of

  while ({ <body> ; <cond> }) ()

Under -rewrite one syntax can be rewritten to the other.

Reasons:

  • do-while is used relatively rarely and it can expressed faithfully using just while. So there seems to be little point in having it as a separate syntax construct.
  • we'd like to reclaim the do as a statement continuation token.
  • Using while as the only loop syntax gives an elegant solution to the "loop-and-a-half" problem. E.g.
var x: Int = 0
while ({
  x = iterator.next
  x >= 0
}) println(x)
odersky added 4 commits Aug 5, 2019
Cleanup errorOrMigrationWarning in Parser
We used three different methods, which all did the same thing in the end.
Drop do-while
Drop . Rewrite to
instead. This can be done by the compiler under -rewrite.
@propensive

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

It seems odd, initially, not to have the statement and predicate wrapped in {}, but (in your example), the braces would also imply a constrained scope of x, so I can see a lot of sense in this, and it's consistent with the bracketless for we now have.

I checked to see if the predicate in an if/then/else could also facilitate a statement, and it can't, though for consistency (not so much for compelling utility), I think it ought to, e.g.

def signed(x: Iterator[Int]) = if
  val h = x.next
  h > 0
then s"+$h" else h.toString

This would help to establish an invariant that new identifiers introduced in expressions comprising of multiple keywords would be scoped to the entire expression.

However, would this work? There's an argument to be made (though I'm not sure I have strong opinions) that a block which evaluates to a Boolean used in an expression position should not have it's scope leak outside the limits of that block.

while {
  val x = iterator.next
  x >= 0
} do println(x)

Another quirk of the new syntax is that a multi-line predicate can be written without braces, whereas a multi-line action cannot, though it seems like the latter would be by far the more common case.

odersky added 2 commits Aug 6, 2019
@odersky

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

@propensive The brace-less syntax in while slipped in by accident. That's not implemented (yet?) Details remain to be worked out. For the moment I have put the braces back.

@propensive

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

@propensive The brace-less syntax in while slipped in by accident. That's not implemented (yet?)

Ok, great. I'll wait and see. :)

@nicolasstucki
Copy link
Contributor

left a comment

Otherwise LGTM

compiler/src/dotty/tools/dotc/parsing/Parsers.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/parsing/Parsers.scala Outdated Show resolved Hide resolved
Apply suggestions from code review
Co-Authored-By: Nicolas Stucki <nicolas.stucki@gmail.com>

@odersky odersky merged commit a0c370a into lampepfl:master Aug 6, 2019

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr Build is passing
Details
atSpan(in.skipToken()) {
in.errorOrMigrationWarning(
i"""`do <body> while <cond>' is no longer supported,
|use `while {<body> ; <cond>} do ()' instead.

This comment has been minimized.

Copy link
@sjrd

sjrd Aug 6, 2019

Member

I'm pretty sure we discussed today that this PR shouldn't be mixed up with that possible future syntax. Hence I believe we should suggest while ({ body; cond }) () here instead.

}
patch(source, cond.span.endPos, "}) ()")
}
WhileDo(Block(body :: Nil, cond), Literal(Constant(())))

This comment has been minimized.

Copy link
@sjrd

sjrd Aug 6, 2019

Member

This is incorrect, as both body and block could define symbols with the same name. If they are put together in a single Block, the scopes will clash.

This comment has been minimized.

Copy link
@nicolasstucki

nicolasstucki Aug 6, 2019

Contributor

We will need a test for this.

This comment has been minimized.

Copy link
@nicolasstucki

nicolasstucki Aug 6, 2019

Contributor

Actually, body and cond are already blocks if they have definitions. I added some tests in #7002. Did I miss some other definitions that could be represented differently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.