[mojo-stdlib] Added fn to normalize index calculation

[StandinKP]

Fixes #2251

[soraros]

Why do we need the _max here?

[StandinKP]

It would be here just to prevent the negative indexes. How would you suggest to do it?

[soraros]

Given that index outside [-len(self), len(self)) is out of bound, we can simply skip the _max call.

return index if index >= 0 else index + len(self)

[StandinKP]

I tried this but it gives a seg fault. I will try to resolve it but till then I think keeping max would be good

[StandinKP]

@JoeLoser @gabrieldemarmiesse can I get your views on this ?

[JoeLoser]

Hey @StandinKP — sorry for the slow reply here. We discussed this the other day during our team meeting and here's my notes:

• The current code seems wrong because it will silently turn an out of bounds negative value into an index of the 0th element by using _max. But I don’t think the solution is to remove the _max and allow negative indices to segfault (or worse) at runtime. Instead, let's abort on out-of-bound indices.
• In the future, it would be nice to have one "normalized index" function for use in the library for any contiguous data structure we have in the library such as List, String, etc. Nothing to do for this PR, but just pointing out how we'd like to clean this up over time.
• In the short (and long-term), I think aborting on out-of-bounds access is a reasonable approach, both here in List and elsewhere.

[StandinKP]

Sure. Makes sense. Do you want me to keep an issue filed to add the common "normalized function" and update all references?

[abduld]

Thanks

[StandinKP]

@JoeLoser @soraros I am facing this issue right now after rebasing with nightly.

❯ ./stdlib/scripts/run-tests.sh
Creating build directory for building the stdlib running the tests in.
Packaging up the Standard Library.
Included from /Users/kaushalphulgirkar/Documents/Projects/mojo/stdlib/src/collections/__init__.mojo:1:
/Users/kaushalphulgirkar/Documents/Projects/mojo/stdlib/src/collections/list.mojo:573:39: error: invalid initialization: could not deduce parameter 'type' of parent struct 'Reference'
        var normalized_idx = Reference(self)._normalize_index(i)
                             ~~~~~~~~~^~~~~~
/Users/kaushalphulgirkar/Documents/Projects/mojo/stdlib/src/__init__.mojo:1:1: note:  struct declared here
# ===----------------------------------------------------------------------=== #
^
Included from /Users/kaushalphulgirkar/Documents/Projects/mojo/stdlib/src/memory/__init__.mojo:1:
/Users/kaushalphulgirkar/Documents/Projects/mojo/stdlib/src/memory/reference.mojo:222:8: note: function declared here
    fn __init__(inout self, value: Self._mlir_type):
       ^
mojo: error: failed to parse the provided Mojo source module

Not able to figure how to exactly fix this. Any suggestions? Did we change the Reference initializations?

[gabrieldemarmiesse]

Hi @StandinKP, indeed you are right :) there was a change with how Reference works right now, see 0fe4cb7

I think that in your case, you don't need to do Reference(self) since self is already a reference. The error message from the compiler is not very clear. Let's hope it will get better in the future!

[StandinKP]

Makes sense. Thanks @gabrieldemarmiesse

[StandinKP]

@gabrieldemarmiesse if I use self[]._normalize_index(i) it gives a very nasty error and crashes all the test suite
trying to figure out this but if you have any idea let me know Thanks