Fixed forRange in Julia for positive step values
#3994
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Julia's ranges include the end value, and other languages don't. I updated the Julia renderer to subtract `1` from the end value to match the other languages. Note that while this fixes it for positive `step` values, it does not for negative `step` values. More work needs to be done here.
B-rando1
requested review from
JacquesCarette,
bmaclach and
smiths
as code owners
B-rando1
changed the title
forRange in Julia for positive step values
|
I'm not sure what's going on with HLint. The error in the log is |
JacquesCarette
requested changes
Jan 1, 2025
| @@ -2,11 +2,11 @@ module VectorTest | |||
|
|
|||
| global v1 = [1.0, 1.5] | |||
| global v2 = [0.0, -1.0] | |||
| for i in 0:1:length(v1) - 1 | |||
| for i in 0:1:length(v1) - 1 - 1 | |||
There was a problem hiding this comment.
This seems odd -- length - 2 ? This looks like it is over-compensating!
There was a problem hiding this comment.
Good catch. I should have noticed that. It looks like I accounted for the range difference in the thunkAssign function, which uses the forRange function. Because this change is upstream of it (most likely the 'correct' place), it was affected. My new commit should remedy that.
Turns out that `thunkAssign` already subtracted `1`, so the new generated code was subtracting it twice. This code should resolve that issue.
JacquesCarette
approved these changes
Jan 1, 2025
|
#3998 should fix that, but I'm waiting for a review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Julia's ranges include the end value, and other languages don't. I updated the Julia renderer to subtract
1from the end value to match the other languages.Note that while this fixes it for positive
stepvalues, it does not for negativestepvalues. More work needs to be done here.Contributes to #3992