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

for loop direction #8

Open
nullun opened this issue Oct 27, 2022 · 4 comments
Open

for loop direction #8

nullun opened this issue Oct 27, 2022 · 4 comments
Labels
enhancement Planned new feature or improvement

Comments

@nullun
Copy link

nullun commented Oct 27, 2022

I noticed that for loops always expect to increment between the start and end values, only breaking when var equals exactly the end value (which could be a problem if it somehow skips over it). However if you wanted to loop from a higher value to a lower value (e.g. swap the order of for n in 1:5: to for n in 5:1:) it results in TEAL that will hit an AVM error when evaluated ("cost budget exceeded"), since it never breaks.

At first I thought maybe I could quickly modify the tealish/__init__.py file to check if start was greater than end, then write "-" instead of "+"... And then I realised they don't have to be defined values at all, and may be unkonwn until runtime.

int x = btoi(Txn.ApplicationArgs[0])
int y = btoi(Txn.ApplicationArgs[1])
for n in x:y:
  pop(n)
end

Should there be an easy way to define or flip the default direction of a for loop without having to write additional logic to work with an incrementing value?

@nullun
Copy link
Author

nullun commented Oct 28, 2022

I was just thinking about this again, and maybe I'm over thinking it and it doesn't need to be this complicated or even need any changing at all... But would an optional step work? For example for n in 5:1:-1:? I'm sure it would be better if it could be a GenericExpression though allowing more than just a signed integer.

@fergalwalsh
Copy link
Collaborator

Good points. I hadn't considered reverse loops. As you say this would be easy if 1:5 was <LiteralInt>:<LiteralInt> because at compile time we could just check which is bigger and use - or + appropriately.
But it is <expression>:expression to allow for flexibility. In theory at run time it could check which is bigger and branch appropriately but that's a lot of extra code. In general I want as much decision making as possible at compile time.

When I read your issue last night I also thought of 5:1:-1 like Python slice steps. Maybe it's a good option if we both came up with it independently :) I think it is slightly less intuitive than being able to do 5:1 but at least for literals the compiler could catch that and warn to change it to specify the step.

Explicitly specifying the step immediately signals to the compiler that this is a reverse loop so it will output the appropriate Teal but also it's a nice signal to the reader.

I don't think the step needs to be an expression. That could allow for very unreadable code for little gain. I'd suggest we have two forms:

for n in start:end:
and
for n in start:end:-1:

If you do for n in start:end:1: it should raise a warning and ideally rewritten to for n in start:end: by the formatter.
If you do for n in start:end:{anything else}: it should be a compile error.

How does that sound?

@fergalwalsh
Copy link
Collaborator

fergalwalsh commented Oct 28, 2022

Maybe a custom step is useful though?
No reason why we couldn't just allow for n in start:end:<SignedLiteralInt>:

So you could do:
for n in 0:100:10
or
for n in 100:0:-10

But still not for n in 0:10:sqrt(20 + 5):

@nullun
Copy link
Author

nullun commented Oct 28, 2022

Yes! Similar to how Python's slice step works. I suspect <SignedLiteralInt> covers the majority of uses people will want to use a for loop for, and if they do require something more complex then using a while loop allows them to implement an expression within the body.

@gokselcoban gokselcoban added the enhancement Planned new feature or improvement label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Planned new feature or improvement
Projects
None yet
Development

No branches or pull requests

3 participants