This change makes LibJS correctly report a syntax error when a unary
expression is followed by exponentiation, as the spec requires.
Apparently this is due to that expression being ambiguous ordering.
Strangely this check does not seem to apply in the same way for '++' and
'--' for reasons that I don't fully understand. For example
```
let x = 5;
++x ** 2
```
Since `--5` and `++5` on it's own results in a syntax error anyway, it
seems we do not need to perform this exponentiation check in those
places.
Diff Tests:
+6 ✅ -6 ❌
The first test crashes in AST, and fails in bytecode, so the best thing
which we can do here without complicated test setup logic is to just
skip this test for now. Interestinglny, this crashing test is very
similar to the existing thenable test case, and only differs in the way
that the thenable is given to the async function.
The next two tests are effectively the same as the above two mentioned
tests, with the only different being that the thenable calls the fulfill
function. For the test case that crashes in AST mode, doing that appears
to fix the test case for AST mode (but both still fail in bytecode).
Previously we were unable to parse code like `yield/2` because `/2`
was parsed as a regex. At the same time `for (a in / b/)` was parsed
as a division.
This is solved by defaulting to division in the lexer, but calling
`force_slash_as_regex()` from the parser whenever an IdentifierName
is parsed as a ReservedWord.
Instead of passing the continuously merged initial forbidden token set
(with the new additional forbidden tokens from each parsed secondary
expression) to the next call of parse_secondary_expression(), keep a
copy of the original set and use it as the base for parsing the next
secondary expression.
This bug prevented us from properly parsing the following expression:
```js
0 ?? 0 ? 0 : 0 || 0
```
...due to LogicalExpression with LogicalOp::NullishCoalescing returning
both DoubleAmpersand and DoublePipe in its forbidden token set.
The following correct AST is now generated:
Program
(Children)
ExpressionStatement
ConditionalExpression
(Test)
LogicalExpression
NumericLiteral 0
??
NumericLiteral 0
(Consequent)
NumericLiteral 0
(Alternate)
LogicalExpression
NumericLiteral 0
||
NumericLiteral 0
An alternate solution I explored was only merging the original forbidden
token set with the one of the last parsed secondary expression which is
then passed to match_secondary_expression(); however that led to an
incorrect AST (note the alternate expression):
Program
(Children)
ExpressionStatement
LogicalExpression
ConditionalExpression
(Test)
LogicalExpression
NumericLiteral 0
??
NumericLiteral 0
(Consequent)
NumericLiteral 0
(Alternate)
NumericLiteral 0
||
NumericLiteral 0
Truth be told, I don't know enough about the inner workings of the
parser to fully explain the difference. AFAICT this patch has no
unintended side effects in its current form though.
Fixes#18087.
This is only visible with something like `Object.getOwnPropertyNames` on
the global object. All other declaration instantiations put the
functions on an environment making the order invisible.
Note that spec order is not quite tree order as in non-strict mode
functions which get hoisted out of blocks appear before top level
functions.
Co-authored-by: Hendiadyoin1 <leon.a@serenityos.org>
The same expression is not allowed to contain both the
logical && and || operators, and the coalescing ?? operator.
This patch changes how "forbidden" tokens are handled, using a
finite set instead of an Vector. This supports much more efficient
merging of the forbidden tokens when propagating forward, and
allowing the return of forbidden tokens to parent contexts.
Although those are the only valid options parse_primary_expression is
sometimes called when only an expression is valid which means it did not
check match_expression and might fail the now removed VERIFY.
In static init blocks 'await' cannot be used. Note that this does not
cover all the cases since the parser currently cannot distinguish
between expressions within parenthesis and direct expressions.
Before this we used an ad-hoc combination of references and 'variables'
stored in a hashmap. This worked in most cases but is not spec like.
Additionally hoisting, dynamically naming functions and scope analysis
was not done properly.
This patch fixes all of that by:
- Implement BindingInitialization for destructuring assignment.
- Implementing a new ScopePusher which tracks the lexical and var
scoped declarations. This hoists functions to the top level if no
lexical declaration name overlaps. Furthermore we do checking of
redeclarations in the ScopePusher now requiring less checks all over
the place.
- Add methods for parsing the directives and statement lists instead
of having that code duplicated in multiple places. This allows
declarations to pushed to the appropriate scope more easily.
- Remove the non spec way of storing 'variables' in
DeclarativeEnvironment and make Reference follow the spec instead of
checking both the bindings and 'variables'.
- Remove all scoping related things from the Interpreter. And instead
use environments as specified by the spec. This also includes fixing
that NativeFunctions did not produce a valid FunctionEnvironment
which could cause issues with callbacks and eval. All
FunctionObjects now have a valid NewFunctionEnvironment
implementation.
- Remove execute_statements from Interpreter and instead use
ASTNode::execute everywhere this simplifies AST.cpp as you no longer
need to worry about which method to call.
- Make ScopeNodes setup their own environment. This uses four
different methods specified by the spec
{Block, Function, Eval, Global}DeclarationInstantiation with the
annexB extensions.
- Implement and use NamedEvaluation where specified.
Additionally there are fixes to things exposed by these changes to eval,
{for, for-in, for-of} loops and assignment.
Finally it also fixes some tests in test-js which where passing before
but not now that we have correct behavior :^).
This commit implements parsing for `yield *expr`, and the multiple
ways something can or can't be parsed like that.
Also makes yield-from a TODO in the bytecode generator.
Behold, the glory of javascript syntax:
```js
// 'yield' = expression in generators.
function* foo() {
yield
*bar; // <- Syntax error here, expression can't start with *
}
// 'yield' = identifier anywhere else.
function foo() {
yield
*bar; // Perfectly fine, this is just `yield * bar`
}
```
