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

certain seq manipulations fail when compiled to JS #10651

Closed
liwt31 opened this issue Feb 12, 2019 · 8 comments

Comments

Projects
None yet
3 participants
@liwt31
Copy link

commented Feb 12, 2019

"certain" is a quite vague word and the best explanation is probably the following example:

Example

var x: seq[int]
var y = @[1,2]
x &= y
echo x[0]

Current Output

The code is compiled to JS and running the script leads to the following error message in the console:

Uncaught TypeError: Cannot read property 'length' of null
add_25021 @ test.js:535
(anonymous) @ test.js:565

The code works perfectly well when compiled to C.
add a single element also works.

Expected Output

1

Additional Information

$ nim -v
Nim Compiler Version 0.19.4 [Linux: amd64]
Compiled at 2019-02-01
Copyright (c) 2006-2018 by Andreas Rumpf

git hash: b6d96cafc8bcad1f3d32f2910b25cd11a93f7751
active boot switches: -d:release

Thanks for your time and effort!

@mratsim

This comment has been minimized.

Copy link
Collaborator

commented Feb 12, 2019

I think that when the nil changes were introduced in sequences around this commit https://github.com/nim-lang/Nim/commits/27f488e5d9ad0e40b23fba7ab1a8b713c823e92c the javascript backend was forgotten.

@Araq

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

@mratsim No, it wasn't forgotten but apparently some issues remained.

@liwt31

This comment has been minimized.

Copy link
Author

commented Feb 13, 2019

I just found another bug relating to seq manipulation. I think it has the same origin with this issue.

Example

proc test =
  var x: seq[int]
  try:
    x.add 1
    echo x.pop
  finally:
    discard

test()

Current Output

The code is compiled to JS and running the script leads to the following error message in the console:

Uncaught Error: Error: unhandled exception: index out of bounds [IndexError]
Traceback (most recent call last)
test.test, line: 5
pop.pop, line: 2525

unhandledException @ test.js:486
raiseException @ test.js:254
raiseIndexError @ test.js:511
chckIndx @ test.js:283
pop_25018 @ test.js:534
test_25003 @ test.js:554
(anonymous) @ test.js:564

The code works perfectly well when compiled to C.
The try block and wrapping the seq in a function call are essential to reproduce the error.

Expected Output

1

Thanks!

@liwt31

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

Yet another bug is found. I hope this can be fixed soon because it's breaking lots of my modules...

Example

var a: seq[int]

a.setLen(0)

echo "ok"

Current Output

The code is compiled to JS and running the script leads to the following error message in the console:

Uncaught TypeError: Cannot read property 'length' of null
    at test.js:86
(anonymous) @ test.js:86

The code works perfectly well when compiled to C.
Thanks!

@liwt31 liwt31 changed the title certain seq manipulation fails when compiled to JS certain seq manipulations fail when compiled to JS Feb 14, 2019

@Araq Araq added the Regression label Feb 14, 2019

@Araq

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

var x: seq[int]
var y = @[1,2]
x &= y
echo x[0]

does work for me.

@liwt31

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

var x: seq[int]
var y = @[1,2]
x &= y
echo x[0]

does work for me.

That's awkward...I tested on another fresh 0.19.4 build in Linux environment and the problem persists. I can see the unchecked nil in the compiled js:

function add_25021(x_25027, x_25027_Idx, y_25030) {
    var F = {
        procname: "add.add",
        prev: framePtr,
        filename: "../../../.choosenim/toolchains/nim-0.19.4/lib/system.nim",
        line: 0
    };
    framePtr = F;
    F.line = 1612;
    var xl_25038 = (x_25027[x_25027_Idx] != null ? x_25027[x_25027_Idx].length : 0);
    F.line = 1613;
    if (x_25027[x_25027_Idx].length < chckRange(addInt(xl_25038, (y_25030 != null ? y_25030.length : 0)), 0, 2147483647)) {
        for (var i = x_25027[x_25027_Idx].length; i < chckRange(addInt(xl_25038, (y_25030 != null ? y_25030.length : 0)), 0, 2147483647); ++i) x_25027[x_25027_Idx].push(0);
    } else {
        x_25027[x_25027_Idx].length = chckRange(addInt(xl_25038, (y_25030 != null ? y_25030.length : 0)), 0, 2147483647);
    };
    L1: do {
        F.line = 1614;
        var i_25073 = 0;
        F.line = 2104;
        var colontmp__25075 = 0;
        F.line = 1614;
        colontmp__25075 = (y_25030 != null ? (y_25030.length - 1) : -1);
        F.line = 2107;
        var res_25078 = 0;
        L2: do {
            F.line = 2108;
            L3: while (true) {
                if (!(res_25078 <= colontmp__25075)) break L3;
                F.line = 2109;
                i_25073 = res_25078;
                F.line = 1614;
                x_25027[x_25027_Idx][chckIndx(addInt(xl_25038, i_25073), 0, x_25027[x_25027_Idx].length + 0 - 1) - 0] = y_25030[chckIndx(i_25073, 0, y_25030.length + 0 - 1) - 0];
                F.line = 2110;
                res_25078 = addInt(res_25078, 1);
            }
        } while (false);
    } while (false);
    framePtr = F.prev;


}

var x_25004 = [null];
var y_25018 = [nimCopy(null, [1, 2], NTI25017)];
add_25021(x_25004, 0, y_25018[0]);
rawEcho(cstrToNimstr((x_25004[0][0]) + ""));

The error comes from x_25027[x_25027_Idx].length, where x_25027[x_25027_Idx] is null in this case.

The compile command is nim js test.nim.

@Araq

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

The JS codegen changed a lot on devel, that's why. The other problems I can reproduce though.

@liwt31

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

Great. I wish I could help but I'm totally unfamiliar with how nim compiler works.

@Araq Araq closed this in b081eb4 Feb 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.