fix: make Ord for slices lexicographic (elements first, then length)

[radik878]

Problem: The Ord implementation for slices [T] compared lengths before elements, contradicting the function comment and common lexicographic expectations. This also diverged from arrays [T; N] and tuples, which are lexicographically ordered. Example of the surprising behavior: [2] was considered less than [1, 1, 1] purely due to length.

Changes: Updated Ord for [T] in noir_stdlib/src/cmp.nr to compare the common prefix element-wise and, only if equal, use length as a tiebreaker. Added unit test asserting [2] > [1,1,1] and [1,2] < [1,2,3].

Goal: Align behavior with documentation and established semantics across the stdlib, ensure predictable lexicographic ordering for slices, and reduce surprises for users.

[github-actions]

Thank you for your contribution to the Noir language.

Please do not force push to this branch after the Noir team have started review of this PR. Doing so will only delay us merging your PR as we will need to start the review process from scratch.

Thanks for your understanding.

[jfecher]

Thank you - good catch 👍

Since the stdlib was changed, I'm expecting each snapshot needs to be updated as well. Do you mind running cargo t && cargo insta review? Make sure cargo insta is installed.

[radik878]

Thank you - good catch 👍

Since the stdlib was changed, I'm expecting each snapshot needs to be updated as well. Do you mind running cargo t && cargo insta review? Make sure cargo insta is installed.

Yes, i will do it, but if you don't mind, i will do it tomorrow

[radik878]

Thank you - good catch 👍

Since the stdlib was changed, I'm expecting each snapshot needs to be updated as well. Do you mind running cargo t && cargo insta review? Make sure cargo insta is installed.

Hi. I fixed tests and now code compiles, but i've bumped into a problem with snapshots:
@radik878 ➜ /workspaces/noir (fix/slice-ord-lexicographic) $ cargo insta review
done: no snapshots to review
@radik878 ➜ /workspaces/noir (fix/slice-ord-lexicographic) $

and to be honest i don't know why it's like that

[jfecher]

@radik878 did you run cargo t first to run all the tests? From the errors above it looks like there are still snapshots which haven't been updated. cargo insta review without running tests first won't do anything since it doesn't have the updated results of each test.

Also, the stdlib needs to be reformatted. You can cd into noir_stdlib and run nargo fmt.

[radik878]

@radik878 did you run cargo t first to run all the tests? From the errors above it looks like there are still snapshots which haven't been updated. cargo insta review without running tests first won't do anything since it doesn't have the updated results of each test.

Also, the stdlib needs to be reformatted. You can cd into noir_stdlib and run nargo fmt.

Thanks for help, looks ready

[jfecher]

@radik878 looks like they are still outdated, maybe the branch was old and you need to git pull origin master and merge in all the changes from master then run cargo t; cargo insta review again?

I'm very sorry this PR has been blocked for as long as it has - we're working on reworking our CI to make this process easier.
If you want, I can copy over these changes into my own branch, run these locally, and make a new PR for it.

[radik878]

@radik878 looks like they are still outdated, maybe the branch was old and you need to git pull origin master and merge in all the changes from master then run cargo t; cargo insta review again?

I'm very sorry this PR has been blocked for as long as it has - we're working on reworking our CI to make this process easier. If you want, I can copy over these changes into my own branch, run these locally, and make a new PR for it.

Thanks, i've bumped into such a system with snaphots for the first time so it was a bit difficult for me, at that moment all tests passed and snapshots updated, but yes, i think branch was outdated. But thanks for help really