JacquesCarette / Drasil

Generate all the things (focusing on research software)
https://jacquescarette.github.io/Drasil
BSD 2-Clause "Simplified" License
142 stars 26 forks source link

Negative Step in `listSlice` (GOOL) #3809

Closed B-rando1 closed 2 months ago

B-rando1 commented 3 months ago

I noticed that in the listSlice function that GOOL uses, a negative step value results in different behaviour between different targets. For example I'll be looking at the following pseudocode:

l = [1, 2, 3, 4]
s = l.slice(start=2, end=0, step=-1)
print(s)

In Python this translates to the nicest (in my opinion) functionality: it prints [3, 2] - that is, it created a slice from index 2 to index 1, reversing the list because of the negative step.

In other languages, this prints [], either because those languages don't support negative steps or because we aren't taking advantage of them.

It also gets even worse when we leave out the start or end values. GOOL allows us to leave either of those out, which has the effect of automatically starting at the start or ending at the end. In Python when these are combined with a negative step, it simply reverses the list. In other languages though, it throws out-of-bounds errors.

It would probably be wise to create some consistency between targets. Our two options are to either assume that the step will never be negative (and ideally enforce this constraint), or bring the other languages up to speed with Python. The second option is probably better, but there are some tricky edge cases we would need to think about.

For example, what does the pseudocode

s = l.slice(, b, c)

translate to? Depending on whether the runtime value of c is positive or negative, we have to start either at 0 or at b, and have to keep going either until the index is greater than b or less than 0. It's not impossible to get this right, but it would result in some messy code.

What are your thoughts? At this point I'm wondering if it might be better to assume that the step will always be positive, and add a listReverse function later if we find we need a way of reversing a list.

JacquesCarette commented 3 months ago

The semantics of negative list slice is supposed to be Python's. If it doesn't work "directly" in other languages, then proper code should be generated instead.

We definitely want to allow to state, at a higher level, what we want, and have the low-level generator 'do the right thing'. GOOL is not supposed to represent code but rather code-like intent.

B-rando1 commented 3 months ago

I did some work on a fix for this yesterday, focusing on Swift. Swift mostly worked how it was supposed to, except that if beg or end wasn't given, they would default to the start or end of the list respectively, instead of checking the sign of step. I added some logic to check step and assign a value to each of beg or end at runtime if it isn't given. This fixes the issue, and has the same behaviour as Python for all 10 of the test cases I created.

The one thing I don't like is that some of these checks don't look smart. For example, code generated for one test:

var endIdx0: Int
if -1 > 0 {
    endIdx0 = myOtherList.count
}
else {
    endIdx0 = -1
}
mySlicedList7 = [Int](stride(from: 3, to: endIdx0, by: -1)).map({(i: Int) -> Double in myOtherList[i]})

It seems like there should be a way within GOOL to tell that -1 is a litInt, and do the logic of finding endIdx within GOOL rather than generating code to do it at runtime. I don't currently know of a way to get the value held by an SValue, though. Would either of you know, @JacquesCarette or @bmaclach? Thanks!

JacquesCarette commented 3 months ago

I forget the exact details, but we definitely changed some of the data-structures to have some 'static' information be available. SValue, if I remember well, is too opaque, and you won't be able to dig it out (as it could be an expression).

You are right that this logic should be possible to do within GOOL. You should look at other places where logic is done in GOOL and see where the information it uses is 'tucked away', and you'll need to mimick that.

bmaclach commented 3 months ago

I think I can fill in some details here. This issue sounds like it's in the same vein as an issue I worked on where we wanted to ensure we generated only the necessary parentheses when rendering expressions. In that case, GOOL had to know when a value was an expression, and if so, it needed to know the precedence of the operation in the expression. I ended up storing that information in the valPrec field of ValData. So I think looking at and understanding how GOOL uses valPrec will demonstrate how a problem of this type can be solved.

smiths commented 3 months ago

Thank you @bmaclach.

balacij commented 3 months ago

Since question is now an actionable ticket, can you please update the OP with tasks that need to be done to close it, @B-rando1 ?

B-rando1 commented 3 months ago

What we need to do in order to close this ticket:

This is going well so far, and has been a fun challenge! I think once the Swift PR is merged, the other changes should be quick.