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

Issues with forRange #3992

Open
B-rando1 opened this issue Dec 28, 2024 · 2 comments
Open

Issues with forRange #3992

B-rando1 opened this issue Dec 28, 2024 · 2 comments
Assignees
Labels

Comments

@B-rando1
Copy link
Collaborator

As I noted in #3991, Julia's ranges include the end value, whereas other languages do not. At first I thought we could get away with a quick fix of subtracting 1 from the given end value, but then I remembered that negative step values exist, and if step is negative we need to add 1 instead.

Furthermore, I looked at other generated code with negative step values (test cases for this do not currently exist; we will need to add some to main) in other langauges and realized that three other languages does not handle negative step values correctly either. Basically it's a similar issue to what we had in #3809, where our current implementations do not account for negative step.

What needs to be done:

  • Julia: if step is positive, subtract 1 from end; if step is negative, add 1 to end.
  • Java, C#, C++: change the inequality checking i so it's < when step is positive and > when step is negative.
  • Python is fine, and Swift appears to be fine as well. I don't have Swift installed on my current machine, but I can install it to double-check at some point.

As with listSlice, we can make it so that if the value of step is known at generation-time, we generate more concise code. Otherwise we need to generate code to do that logic at runtime.

@B-rando1 B-rando1 added the bug label Dec 28, 2024
@B-rando1 B-rando1 self-assigned this Dec 28, 2024
@JacquesCarette
Copy link
Owner

It feels to me that we need to look more deeply into what forRange means (and ought to mean) to make the rendering in each language more transparent.

I definitely agree with the last paragraph: we need to be clearer about what information is known when (this is something that @Xinlu-Y was working on, but it got very complicated rather quickly). We need to 'teach' our representations more about staging.

@B-rando1
Copy link
Collaborator Author

I made a PR to fix Julia's forRange for positive step values. Beyond that, there seems to be something of a rabbit-hole, so it might be best to leave it for now.

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

2 participants