Boa: [Easy] - increment operator is broken

[jasonwilliams]

let i = 0;
i++;
i;

let i = 0;
++i;
i;

This JS generates the error:
thread 'main' panicked at 'internal error: entered unreachable code', src/lib/exec.rs:246:26 note: run withRUST_BACKTRACE=1environment variable to display a backtrace. The terminal process terminated with exit code: 101

Issue is happening here:
https://github.com/jasonwilliams/boa/blob/master/src/lib/exec.rs#L249

Looks like more unaryOps need to be added including the increment, decrement.
https://github.com/jasonwilliams/boa/blob/master/src/lib/syntax/ast/op.rs#L47-L64

Debugging:
https://github.com/jasonwilliams/boa/blob/master/docs/debugging.md

[dmfutcher]

I'm interested in taking a look at this for Hacktoberfest.

[dmfutcher]

There's a related parsing issue, where i++ is parsed as IncrementPost(IncrementPre(i))

def: Block(
        [
            Expr {
                def: LetDecl(
                    [
                        (
                            "i",
                            Some(
                                Expr {
                                    def: Const(
                                        Num(
                                            0.0,
                                        ),
                                    ),
                                },
                            ),
                        ),
                    ],
                ),
            },
            Expr {
                def: UnaryOp(
                    IncrementPost,
                    Expr {
                        def: UnaryOp(
                            IncrementPre,
                            Expr {
                                def: Local(
                                    "i",
                                ),
                            },
                        ),
                    },
                ),
            },
        ],
    ),
}

[jasonwilliams]

That’s odd, would need to see the tokens.

https://github.com/jasonwilliams/boa/blob/master/docs/debugging.md#tokens should help

[dmfutcher]

JS:

let i = 0;
i++
i

Tokens:

[src/lib/lib.rs:52] lexer.tokens = [
    Token {
        data: Keyword(
            Let,
        ),
        pos: Position {
            column_number: 1,
            line_number: 1,
        },
    },
    Token {
        data: Identifier(
            "i",
        ),
        pos: Position {
            column_number: 5,
            line_number: 1,
        },
    },
    Token {
        data: Punctuator(
            Assign,
        ),
        pos: Position {
            column_number: 7,
            line_number: 1,
        },
    },
    Token {
        data: NumericLiteral(
            0.0,
        ),
        pos: Position {
            column_number: 9,
            line_number: 1,
        },
    },
    Token {
        data: Punctuator(
            Semicolon,
        ),
        pos: Position {
            column_number: 10,
            line_number: 1,
        },
    },
    Token {
        data: Identifier(
            "i",
        ),
        pos: Position {
            column_number: 1,
            line_number: 2,
        },
    },
    Token {
        data: Punctuator(
            Inc,
        ),
        pos: Position {
            column_number: 2,
            line_number: 2,
        },
    },
    Token {
        data: Identifier(
            "i",
        ),
        pos: Position {
            column_number: 1,
            line_number: 3,
        },
    },
]

The tokens look reasonable from a quick glance. I've not had a proper dive into the parser but parse looks to emit the IncrementPre and parse_next the IncrementPost.

[jasonwilliams]

There should be a second semicolon after the Inc

[jasonwilliams]

Looks like it hits https://github.com/jasonwilliams/boa/blob/master/src/lib/syntax/parser.rs#L765-L770 first, which then calls parse again, that goes into https://github.com/jasonwilliams/boa/blob/master/src/lib/syntax/parser.rs#L565-L568

Because the previous token as an identifier, it will call parse_next() after parsing that.
parse_next falls into Post, Post (first link) doesn't look like it advances the position. There needs to be a self.pos += 1 somewhere

[jasonwilliams]

Taking a closer look, it looks like postIncrement was copied from preIncrement.

PreIncrement tries to parse the next token which is correct, but postIncrement shouldn’t do that, it already knows what it’s incrementing.
It should be taking the previous token (the identifier) and passing that as the argument (as a localExpr).

That’s parsing, for execution, the executor will need to return the current value, and then increment it, this should be fairly straightforward.

Let me know if none of this makes sense

[dmfutcher]

@jasonwilliams, thanks for the tips, will take a proper look at this as soon as possible.

[jasonwilliams]

@bobbo how was this going? need help?

[jasonwilliams]

Closing as the parsing side is fixed the rest will be implemented in https://github.com/jasonwilliams/boa/issues/321