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

Erroneous errors for multiline statements that include BigInt literal suffix #25835

Closed
MatteBru opened this issue Jan 30, 2019 · 5 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.

Comments

@MatteBru
Copy link

MatteBru commented Jan 30, 2019

  • Version: v10.15.0, v10.15.1, v11.8.0
  • Platform: Linux 4.15.0-43-generic # 46~16.04.1-Ubuntu SMP Fri Dec 7 13:31:08 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: REPL

Possibly related to the issues discussed in #24231

When attempting to create a multiline statement that includes the BigInt literal suffix syntax (i.e. Number followed by n to denote a BigInt) causes an erroneous SyntaxError: Invalid or unexpected token to be thrown. This behavior appears to occur only when the end of the statement is not on the same line as the BigInt. For example, the following seems to be erroneous behavior:

> let a = [
... 4,
... 'hello',
... 0xffn,
Thrown:
0xffn,
     ^

SyntaxError: Unexpected end of input

Whereas if the statement ends on the same line, it seems to behave as expected:

> [
... 4,
... 'hello',
... 0xffn]
[ 4, 'hello', 255n ]

The same behavior seems to extend to any multiline statements regardless of whether or not the BigNum is the last thing on the line:

With BigNum:

> function fun(){
... let m = 0xffn; return m;
Thrown:
let m = 0xffn; return m;
                       ^

SyntaxError: Unexpected end of input

Without BigNum:

function fun(){
... let m = 0xff; return m;
... }
undefined

BigNum on same line as closing bracket:

> function fun(){
... let m = 0xffn; return m;}
undefined
> fun()
255n

Edit (Not sure if helpful):

BigInt suffix seems to be picked up as an identifier. Given the following input into REPL:

> [
... 0xffn,

<debugger paused on pressing return key>

pp$4.raise() at line 2753 in internal/deps/acorn/dist/acorn.js is receiving pos = 6, message = "Identifier directly after number (2:4)

That in turn seems to be coming from pp$8.readRadixNumber() at line 4981 in internal/deps/acorn/dist/acorn.js

The issue seems to originate at isIdentifierStart() (internal/deps/acorn/dist/acorn.js:71), which does in fact pick up n as the start of an identifier.

isIdentifierStart() is also called by pp$8.readNumber() when using a normal, non-radix-prefixed number, producing the same result.

isIdentifierStart() is bypassed when the statement is terminated on the same line as the BigInt, which explains why this scenario does not trigger the error:

> [
... 0xffn]
[ 255n ]
>
@addaleax addaleax added the repl Issues and PRs related to the REPL subsystem. label Jan 30, 2019
@targos
Copy link
Member

targos commented Jan 31, 2019

The problem is that acorn doesn't support BigInt literals at the moment. The error only happens when the statement is unfinished because we only try to parse the code with acorn if there is an error.

@targos
Copy link
Member

targos commented Feb 10, 2019

@nodejs/repl should we close this as a known limitation (Node.js, through V8, can support newer syntax that acorn doesn't know about)?

@nickkolok
Copy link

This problem looks really annoying, so +1 for fixing it (here or somewhere else). Have anybody reported it to acorn? What is acorn`?

@BridgeAR
Copy link
Member

BridgeAR commented Mar 4, 2019

I agree with @targos that this is a known limitation and we should not try to fix this on our side. acorn should support BigInt as soon as it officially landed in the spec. As such it's just a matter of time and we should not add special code that we have to remove later on just for this case.

I am going to close this issue for now. If someone disagrees, please leave a note or just reopen the issue.

@BridgeAR BridgeAR closed this as completed Mar 4, 2019
@BridgeAR BridgeAR added the wontfix Issues that will not be fixed. label Mar 4, 2019
@BridgeAR
Copy link
Member

I am actually about to fix this.

@BridgeAR BridgeAR reopened this Apr 24, 2019
@BridgeAR BridgeAR removed the wontfix Issues that will not be fixed. label Apr 24, 2019
@BridgeAR BridgeAR added the confirmed-bug Issues with confirmed bugs. label Apr 27, 2019
BridgeAR added a commit to BridgeAR/node that referenced this issue Apr 30, 2019
This adds bigint, class-fields, numeric-separators, static-class
features, private class methods and fields as dependency. That way
it's possible to use these in combination with acorn to parse these
language features.

This also removes a couple of files that were not necessary for
Node.js to reduce the code base.

PR-URL: nodejs#27400
Refs: nodejs#27391
Refs: nodejs#25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this issue Apr 30, 2019
This adds new language features to acorn. Otherwise BigInt and other
input would not be parsed correct and would not result in nice error
messages when using simple assert.

PR-URL: nodejs#27400
Refs: nodejs#27391
Refs: nodejs#25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this issue Apr 30, 2019
This adds stage-3 language features to acorn so that it's possible
to parse these features when using top level await in the REPL.

PR-URL: nodejs#27400
Refs: nodejs#27391
Refs: nodejs#25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this issue Apr 30, 2019
This adds bigint, class-fields, numeric-separators, static-class
features, private class methods and fields as dependency. That way
it's possible to use these in combination with acorn to parse these
language features.

This also removes a couple of files that were not necessary for
Node.js to reduce the code base.

PR-URL: #27400
Refs: #27391
Refs: #25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this issue Apr 30, 2019
This adds new language features to acorn. Otherwise BigInt and other
input would not be parsed correct and would not result in nice error
messages when using simple assert.

PR-URL: #27400
Refs: #27391
Refs: #25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this issue Apr 30, 2019
This adds stage-3 language features to acorn so that it's possible
to parse these features when using top level await in the REPL.

PR-URL: #27400
Refs: #27391
Refs: #25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this issue Apr 30, 2019
This adds stage-3 language features to acorn so that the REPL is
able to parse these features properly. Otherwise these would cause
SyntaxErrors.

PR-URL: #27400
Fixes: #27391
Fixes: #25835
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
5 participants