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

A statement pulled out of the condition of a while loop is only evaluated once #1362

Closed
Kodiologist opened this issue Aug 4, 2017 · 9 comments
Labels

Comments

@Kodiologist
Copy link
Member

Thinking about #1342 made me guess that the following would do the wrong thing, and indeed, it does:

$ echo '(setv x 3) (while (do (print "hi") x) (print x) (-= x 1))' | hy2py
x = 3
print('hi')
while x:
    print(x)
    x -= 1

The print('hi') only happens once instead of every time the while condition is checked, because it's pulled out to before the while.

The most straightforward fix is to make a while loop with a complex condition compile to something like:

x = 3
while True:
    # Condition
    print('hi')
    _hy_anon_var_1 = x
    if not _hy_anon_var_1:
        break
    # Body
    print(x)
    x -= 1

(In practice, Result.rename would remove the use of _hy_anon_var_1 and just write if not x:.)

The problem with this method is that an else clause won't be executed when the loop terminates normally, because break skips the else. This doesn't actually matter right now because we haven't yet implemented else for while. But a method that's robust to this future added feature is:

x = 3
# Condition
print('hi')
_hy_anon_var_1 = x
while _hy_anon_var_1:
    # Body
    print(x)
    x -= 1
    # Condition, repeated
    print('hi')
    _hy_anon_var_1 = x

Note that it will then be unwise to use break or continue inside the condition, because the condition executes once outside the while and subsequently inside it.

@gilch
Copy link
Member

gilch commented Aug 4, 2017

How should a break/continue work in the condition? I think these statements are only supposed to be valid in a loop body. We'd get a SyntaxError: 'break' outside loop before we even got to the inner one. And this is fine.

The problem with the second option is what happens with nested loops. If the while with a break in the condition is nested inside another loop, then it will break the outer loop on the first iteration, but the inner loop after that, which is weird and inconsistent. Similarly with continue.

The first option only puts the condition inside the body, so it will only break the inner loop, which is still weird, but at least it's consistent.

We could probably disallow (break) or (continue) inside a while condition in Hy's compiler altogether. I don't see a use for it anyway. I'm not sure how easily we could detect this though. We'd need a context stack, since another while should be allowed in the condition, and there's nothing wrong with putting a break in that.

Maybe there's some other way to compile this that gives us sensible break/continue without corrupting else.

@gilch
Copy link
Member

gilch commented Aug 4, 2017

A third option.

x = 3
_hy_anon_var_1 = True
while _hy_anon_var_1:
    # Condition
    print('hi')
    _hy_anon_var_2 = x
    if _hy_anon_var_2:
        # Body
        print(x)
        x -= 1
    else:
        _hy_anon_var_1 = False

This is like the first option, but we don't add our own break, so the else clause for the while should work properly. The condition also always happens inside the loop, so break/continue are consistent.

[Edit: we don't need two anon vars]

x = 3
_hy_anon_var_1 = True
while _hy_anon_var_1:
    # Condition
    print('hi')
    _hy_anon_var_1 = x
    if _hy_anon_var_1:
        # Body
        print(x)
        x -= 1

@Kodiologist
Copy link
Member Author

Kodiologist commented Aug 4, 2017

If the while with a break in the condition is nested inside another loop, then it will break the outer loop on the first iteration, but the outer loop after that, which is weird and inconsistent.

Right, that's what I was thinking.

I'm not too worried about this in any case because putting a loop-control statement in a loop condition is a weird thing to do in the first place. "Doctor, it hurts when I do this", etc.

Here's another version I thought of that's similar to your version:

x = 3
_hy_anon_var_1 = True
while _hy_anon_var_1:
    # Condition
    print('hi')
    _hy_anon_var_1 = bool(x)  
    if _hy_anon_var_1:
        # Body
        print(x)
        x -= 1

The bool is so the anon var stays the same even if the truth value of the object referred to by x changes (e.g., if x is a list and it's emptied).

@Kodiologist
Copy link
Member Author

Oh, jinx!

@gilch
Copy link
Member

gilch commented Aug 4, 2017

Haha. I realized I didn't need two anon vars so I edited it to simplify. I put the original version back for posterity.

@gilch
Copy link
Member

gilch commented Aug 4, 2017

Good point about using bool. A mutation in the body could prematurely interrupt the loop, without executing the condition clause again. But since Python builtins can be shadowed in a module, I'd prefer not to rely on them in the compiler when they aren't explicitly asked for, if possible. If we have to use a builtin, it should compile like--

from builtins import bool as _hy_anon_var_2  # from __builtin__ in Python2
_hy_anon_var_1 = _hy_anon_var_2(x)

But in this case, we could avoid it altogether like this:

x = 3
_hy_anon_var_1 = True
while _hy_anon_var_1:
    # Condition
    print('hi')
    _hy_anon_var_1 = x
    if _hy_anon_var_1:
        _hy_anon_var_1 = True
        # Body
        print(x)
        x -= 1

@Kodiologist
Copy link
Member Author

True. You could also write not not x.

@trylks
Copy link

trylks commented Aug 23, 2017

Hi, first: great job here with Hy. I'm looking forward for version 1.0.

About this issue, why not creating a local function?

x=3
def _hy_anon_def_1():
  print('hi')
  return x
while _hy_anon_def_1():
  print(x)
  x -= 1

Does that mess up with the AST too much?

@Kodiologist
Copy link
Member Author

Kodiologist commented Aug 23, 2017

We don't create anonymous functions when we don't have to because each function creates a new scope, meaning that setv won't set the variable that the user expects. Also, now that we have return, we need to make sure it returns from the right function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants