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

IndexError when use a list of funcs generated bymapit #12625

Closed
foldl opened this issue Nov 8, 2019 · 6 comments
Closed

IndexError when use a list of funcs generated bymapit #12625

foldl opened this issue Nov 8, 2019 · 6 comments
Labels

Comments

@foldl
Copy link
Contributor

foldl commented Nov 8, 2019

Use mapit to generate a list of functions. When calling these functions, Index error occur.

Example

import sequtils, sugar
static:
  # following map & mapit should be equivalent
  echo [1, 2].map((it) => func (x: int): int = it + x).map((f) => f(100))
  echo [1, 2].mapit(func (x: int): int = it + x).map((f) => f(100))

Current Output

@[101, 102]
fatal.nim(39)            sysFatal
Error: unhandled exception: index 23 not in 0 .. 2 [IndexError]

Expected Output

@[101, 102]
@[101, 102]

Possible Solution

N/A.

Additional Information

N/A

$ nim -v
Nim Compiler Version 1.0.2 [Windows: amd64]
Compiled at 2019-10-22
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: 193b3c66bbeffafaebff166d24b9866f1eaaac0e
active boot switches: -d:release
@narimiran
Copy link
Member

Regarding the expected output, running the example outside of the static block gives:

@[101, 102]
@[102, 102]

@foldl
Copy link
Contributor Author

foldl commented Nov 11, 2019

Here is an example that can't give the correct result running in isMainModule.
It seems like that only m[1] is used in mapIt.

if isMainModule:
  let m = @[func (x: int): int = x + 100, func (x: int): int = x + 200]
  let l = m.mapIt(func (x: int): int = it(x))
  echo l.mapIt(it 1)

Current Output

@[201, 201]

Expected Output

@[101, 201]

@alehander92
Copy link
Contributor

hm, is if isMainModule valid , i thought one can only when

@narimiran
Copy link
Member

hm, is if isMainModule valid , i thought one can only when

It doesn't matter, I originally reproduced the problem without it.

@foldl
Copy link
Contributor Author

foldl commented Nov 13, 2019

My finding: it is not captured by the generated proc of mapIt, but defined as
a global variable.

So, the title of this issue should be changed to: {.inject} is not handled properly when
generating closure?

if isMainModule:
  let m = @[func (x: int): int = x + 100, func (x: int): int = x + 200]
  # The "simplified" generated C code of this line is listed below
  let l = m.mapIt(func (x: int): int = it(x))  
  echo l.mapIt(it 1)

The "simplified" generated C code:

it__MpefkJ9cQAwQD9aL17D5DVvw; // *it* is a global variable, which is wrong!
N_LIB_PRIVATE N_NIMCALL(NI, colonanonymous___vsp4xqNMQqR9b4HAKLhepJw_3)(NI x) {	NI result;
	result = it__MpefkJ9cQAwQD9aL17D5DVvw(x);
	return result;
}

// let l = m.mapIt(func (x: int): int = it(x))
for (int i = 0; i < 2; i++)
{
    it__MpefkJ9cQAwQD9aL17D5DVvw = m[i];
    l[i].proc = colonanonymous___vsp4xqNMQqR9b4HAKLhepJw_3;
    l[i].env = NIL;
}

foldl added a commit to foldl/Nim that referenced this issue Nov 20, 2019
This fixes part of the issue nim-lang#12625. vm & other backends also need
update.
foldl added a commit to foldl/Nim that referenced this issue Nov 20, 2019
This fixes part of the issue nim-lang#12625. vm & other backends also need
update.
@foldl
Copy link
Contributor Author

foldl commented Nov 21, 2019

Some updates on this issue.

Further checking indicates that the root cause is, in a for (or while) loop, env is owned by the proc, so it is initialized out of for. The generated func-s in the loop are all referring to the env by ref, so they are all referencing to the same env, which is modified in each iteration of for.
(This has nothing to do with {.inject.})

@Araq pointed out in this thread:

That's actually what the spec says needs to be done (but I'm not sure it's written down somewhere in the manual. sigh). Reason: Performance, JS interop. Workaround: use system.closureScope instead.

Unfortunately, this workaround does not work for this case. In the proc generated by closureScope, asgnRef is used, failing to capture the local variable correctly.

foldl added a commit to foldl/Nim that referenced this issue Apr 21, 2020
1. fallback to call `map` when the result of `op` is a closure;
2. use `items(s)` in the for loop.
Araq pushed a commit that referenced this issue Apr 21, 2020
* fix mapIt issues #12625 & #12639:

1. fallback to call `map` when the result of `op` is a closure;
2. use `items(s)` in the for loop.

* fix test errors.

* add comments and InType is moved.

* fix ident.
@Araq Araq closed this as completed Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants