-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Drop do while #6994
Conversation
We used three different methods, which all did the same thing in the end.
Drop . Rewrite to instead. This can be done by the compiler under -rewrite.
It seems odd, initially, not to have the statement and predicate wrapped in I checked to see if the predicate in an
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
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. |
@propensive The brace-less syntax in |
Ok, great. I'll wait and see. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
Co-Authored-By: Nicolas Stucki <[email protected]>
atSpan(in.skipToken()) { | ||
in.errorOrMigrationWarning( | ||
i"""`do <body> while <cond>' is no longer supported, | ||
|use `while {<body> ; <cond>} do ()' instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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(()))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need a test for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
Drop
in favor of
Under -rewrite one syntax can be rewritten to the other.
Reasons:
while
. So there seems to be little point in having it as a separate syntax construct.do
as a statement continuation token.while
as the only loop syntax gives an elegant solution to the "loop-and-a-half" problem. E.g.