Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

foldl(2)/foldr(2): support openArray #24628

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

foldl
Copy link
Contributor

@foldl foldl commented Jan 20, 2025

foldl*(sequence, operation, first) supports openArray, so, it would be nice and consistant that foldl(2) also supports it. This PR does this.

But, I am not sure if this is an issue of the compiler (Nim Compiler Version 2.2.0 [Windows: amd64]):

let s = sequence   # when sequence is an openArray, compiler complains 

If it's an issue of the compiler, than this PR is not needed.

So as foldr(2).

@ringabout
Copy link
Member

2025-01-21T09:47:44.2138650Z �[1m�[36mTest "tests/stdlib/tsequtils.nim" in 
2025-01-21T09:47:44.2142510Z Output:
2025-01-21T09:47:44.2142770Z Traceback (most recent call last)
2025-01-21T09:47:44.2143060Z tsequtils.nim(478)       tsequtils
2025-01-21T09:47:44.2143340Z system.nim(936)          concat
2025-01-21T09:47:44.2143620Z arc.nim(230)             nimDecRefIsLast
2025-01-21T09:47:44.2143980Z SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Test failures seem to be related

@foldl
Copy link
Contributor Author

foldl commented Jan 21, 2025

I found that let s = sequence is OK on devel branch when sequence is an openArray.
So this PR is not needed.

And, test failures are bugs of the compiler.

@foldl foldl closed this Jan 21, 2025
@foldl
Copy link
Contributor Author

foldl commented Jan 21, 2025

Oh, not exactly. Sign.

@foldl foldl reopened this Jan 21, 2025
@foldl foldl changed the title foldl(2): support openArray foldl(2)/foldr(2): support openArray Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants