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

[REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries #8188

Closed
timotheecour opened this issue Jul 2, 2018 · 9 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Jul 2, 2018

Here's a simplified scenario.
It's a major blocker for any kind of lazy map/filter library, such as https://github.com/def-/nim-iterutils/blob/53f4a30e57f01cd8cf9d5b1dd22053eac82b25f2/src/iterutils.nim#L303 "TODO: Currently fail" where it shows map/filter etc cannot be composed.

proc myiter*(n:int): auto =
  iterator it: int {.closure.} =
    for x in 0..n:
      yield x
  return it

proc iterFun*[T](a:T): auto =
  iterator it: int {.closure.} =
    for x in a():
      yield x
  return it

proc test()=
  let it1 = myiter(5)
  let it2 = iterFun(it1)

  # nothing is printed here:
  for a in it2(): echo a

  echo "done"

test()

this prints "done"
it should print 0 1 2 3 4 5 done

EDIT this is a regression, I just verified this worked in 0.9.4 (which was nimrod)

$git_clone_D/temp/nim/Nim3/bin/nimrod c -r t26_iterator_compose2.nim
0
1
2
3
4
5
done

$git_clone_D/temp/nim/Nim3/bin/nimrod --version
Nimrod Compiler Version 0.9.4 (2018-07-02) [MacOSX: amd64]
Copyright (c) 2006-2014 by Andreas Rumpf

NOTE: I also verified https://github.com/def-/nim-iterutils used to work with 0.9.4:

git clone https://github.com/def-/nim-iterutils
cd nim-iterutils
git checkout 40b83d2de580b93c5ef96100a72df1ea4981a7a8
$git_clone_D/temp/nim/Nim3/bin/nimrod c -r src/iterutils.nim

NOTE: to build 0.9.4, unfortunately choosenim didn't work, see but I posted in dom96/choosenim#66 (I show there how I built 0.9.4 , not sure if there's a simpler way)

EDIT
I just tried with v0.13.0, that version already has the regression.

Not sure how to build nim for versions between v0.9.4 and v0.13.0 because https://github.com/nim-lang/csources.git doesn't have any git tag between these and ./bin/nimrod c koch fails for csources.git:v0.9.4 with Nim:v0.12.0

EDIT fixed in c554c2a

@timotheecour timotheecour changed the title iterator composition doesn't work iterator composition doesn't work, preventing lazy map/filter/etc libraries Jul 2, 2018
@timotheecour timotheecour changed the title iterator composition doesn't work, preventing lazy map/filter/etc libraries [REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries Jul 2, 2018
@Araq
Copy link
Member

Araq commented Jul 2, 2018

I doubt git bisect will get you anywhere even if you had all the versions in between. Bugs can also be fixed by looking at today's compiler. IIRC this bug is a subtle phase-ordering problem in the closure iterator pass.

@timotheecour
Copy link
Member Author

I doubt git bisect will get you anywhere even if you had all the versions in between. Bugs can also be fixed by looking at today's compiler.

well, knowing what commit broke this would definitely help (especially for ppl not familiar with compiler internals), as it narrows down the search to just a few lines of code

I would consider this a high priority bug as it prevents functional programming based on map/filter/reduce/etc (and breaks a bunch of libraries that used to work, albeit a long time ago)

@Araq
Copy link
Member

Araq commented Jul 2, 2018

#3837 I agreed with you. Duplicate bug, closing.

@Araq Araq closed this as completed Jul 2, 2018
@mratsim
Copy link
Collaborator

mratsim commented Jul 3, 2018

@timotheecour I suggest you use zero-functional which should cover most of your needs with iterators chaining and be significantly faster than closure iterators and also works today (inline iterator chaining is also broken even though it should work according to manual #4516).

One limitation is that you cannot do an infinite iterator but you can initialise from a closure iterator for that.

@timotheecour
Copy link
Member Author

timotheecour commented Jul 13, 2018

@mratsim it's not so much infinite iterator that's not working but more generally iterators from which we don't know the size ahead of time (analog of D's Input range, see https://dlang.org/phobos/std_range_primitives.html). These are fundamental for efficient stream processing (as opposed to batch processing), eg, processing stdin line by line and applying map/filter etc.

/cc @alehander42
Can you confirm that's currently not possible using zero-functional? if it is, could you provide an example for how to do:
(pseudocode)
stdin.byLine.filterIt(it.startsWith("foo")).map(toUpper).foldl(myFun) ?
(note: without using for )

@mratsim
Copy link
Collaborator

mratsim commented Jul 17, 2018

@alehander42 ^

@alehander92
Copy link
Contributor

@timotheecour that should be possible, it would be easy if one already has a defined byLine zero-functional primitive, I think something like stdin --> byLine() --> filter(it.startsWith("foo") --> map(it.toUpper) --> fold(myFun(a, it))

if there is not something like byLine, there is already a nice zf_inline dsl to create a custom one by @michael72

@michael72
Copy link

I don't quite understand the deal?!

proc test() =
  readFile(fileName).splitLines -->> filter("error" in it) --> createIter(s)
  for line in s():
    echo(line)

or

lines(stdin) --> map(it.toUpper()) --> createIter(s)
...

should work?!

@timotheecour timotheecour changed the title [REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries [TODO] [REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries Jul 19, 2018
@timotheecour
Copy link
Member Author

fixed in c554c2a

@timotheecour timotheecour changed the title [TODO] [REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries [REGRESSION] iterator composition doesn't work, preventing lazy map/filter/etc libraries Sep 22, 2018
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

No branches or pull requests

5 participants