nac3core: fix len on range with step of different sign #129

ychenfo · 2021-12-12T06:33:39+08:00

ychenfo commented

2021-12-12 06:33:39 +08:00

Previously I did not consider clearly about all the cases when the step is negative. In this PR I updated the formula to be, which I think should be fine..

def length(start, end, step != 0):
    diff = end - start
    # if diff == 0 OR `diff` and `step` are of different signs, always zero
    if diff * step <= 0:
        return 0
    else:
        return ((abs(diff) - 1) // abs(step)) + 1

previously the formula is just ((abs(diff) - 1) // abs(step)) + 1

Previously I did not consider clearly about all the cases when the step is negative. In this PR I updated the formula to be, which I think should be fine.. ```python def length(start, end, step != 0): diff = end - start # if diff == 0 OR `diff` and `step` are of different signs, always zero if diff * step <= 0: return 0 else: return ((abs(diff) - 1) // abs(step)) + 1 ``` previously the formula is just `((abs(diff) - 1) // abs(step)) + 1`

ychenfo added 1 commit 2021-12-12 06:33:39 +08:00

1a9f39ee4a nac3core: fix len on range with step of different sign

sb10q commented

2021-12-12 10:48:59 +08:00

which I think should be fine.

You don't have to guess. Just adapt the cpython implementation:
https://github.com/python/cpython/blob/main/Objects/rangeobject.c#L182

> which I think should be fine. You don't have to guess. Just adapt the cpython implementation: https://github.com/python/cpython/blob/main/Objects/rangeobject.c#L182

sb10q commented

2021-12-12 11:03:29 +08:00

And RustPython certainly has one as well. Maybe you could even get the LLVM IR out of rustc and essentially copy it.

ychenfo added 1 commit 2021-12-13 04:04:06 +08:00

a3f7d404fb nac3core: use official implementation for len

ychenfo added 1 commit 2021-12-13 04:06:52 +08:00

1541188722 nac3standalone: add output_long

ychenfo commented

2021-12-13 04:16:15 +08:00

Thanks a lot for the information! I implemented again using this now.

Thanks a lot for the information! I implemented again using [this](https://github.com/python/cpython/blob/main/Objects/rangeobject.c#L889-L912) now.

sb10q merged commit ccfcba4066 into master