close: #3969close: #2023
We need to add scope according to [this](d8086f14b6/src/compiler/binder.ts (L3883-L3962)). There are still some ASTs that need to be added to the scope.
---
Context from @Boshen:
Before this whole journey of fixing symbols and scopes I asked @Dunqing to debug through binder.ts via a debugger to fully understand how it does resolution.
We then agreed to align the implementation so that when a problem occurs, we can debug through both implementations and find where our problem is.
tsc doesn't have a specification, so we need to align with the reference implementation instead.
I use this in my local environment when I'm developing the ast_codegen
and want a thorough build with the newly generated files. If you find it
useful merge otherwise feel free to close.
This tweaks the `no-dupe-keys` lint in order to try to optimize it.
The lints reliably shows up in both CPU and memory profiling,
due to the cost of allocating/growing the hashmap and hashing into it.
This PR tries to tackle both by skipping trivial cases and by
pre-allocating a larger hashmap.
We have a strange workaround for `visit_function` where we pass in `ScopeFlags`, to support creating the scope inside `Function`, but setting different flags for `MethodDefinition`s.
Previously `visit_function` took `Option<ScopeFlags>` and then did `flags.unwrap_or(ScopeFlags::empty()) | ScopeFlags::Function` to it. Personally, I found this confusing. When I was looking at `MethodDefinition`, I was wondering "It's a function, why doesn't it set Function flag too?"
This changes makes it more explicit and clear what `ScopeFlags` everything has.
Instead of calling `bind_function_or_class_expression` for every scope, and then branching on the AST node kind, insert the relevant code into the visitors for functions and classes. This reduces work on all other kinds of scopes e.g. block statements.
Previously:
```rs
let ix = control_flow!(|self, cfg| cfg.current_node_ix);
```
after this PR:
```rs
let ix = control_flow!(self, |cfg| cfg.current_node_ix);
```
It expands to:
```rs
let ix = if let Some(ref mut cfg) = self.cfg {
cfg.current_node_ix
} else {
Default::default()
};
```
So rationale for this change is that it makes it clearer that `self` is passed *in* and `cfg` comes *out* into the "closure".
`AstNodeId` was a `usize`. This seems excessive.
Parser has a limit on size of a JS file of 4 GiB. While it is *possible* for a JS file of that size to create an AST with more than `1 << 32` (~4 billion) AST nodes, that would be insanely large.
So make `AstNodeId` `u32` instead.
Control flow graph builder records AST node IDs in a temp structure `ast_nodes_records`. Only the first AST node ID inserted into the `Vec` is ever used, so turn it into just a single value.
Using `AstNodeId::dummy()` as sentinel for "not set" instead of using `Option<AstNodeId>` to reduce size of the records stack. `Option<AstNodeId>` is 16 bytes.
When parsing a number which contains `_` separators, rather than looping
through the string once to remove separators, and then again to convert
to an `f64`, do it all in a single loop.
Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Fix tests for parsing large numbers.
* Tests for imprecision decimals and octals weren't using numbers larger than `u64::MAX`, so were not testing what they were meant to be - that parser can handle larger numbers.
* Tests for decimals (e.g. `123.5`) were calling `parse_int` which isn't meant to handle that (`parse_float` does that).
* Tests were calling `parse_int` with negative numbers, which it's not meant to handle.
Also, re-order the const assertions in same order as the code above.
[](https://renovatebot.com)
This PR contains the following updates:
| Package | Type | Update | Change |
|---|---|---|---|
| [crate-ci/typos](https://togithub.com/crate-ci/typos) | action | patch
| `v1.23.1` -> `v1.23.2` |
---
### Release Notes
<details>
<summary>crate-ci/typos (crate-ci/typos)</summary>
###
[`v1.23.2`](https://togithub.com/crate-ci/typos/releases/tag/v1.23.2)
[Compare
Source](https://togithub.com/crate-ci/typos/compare/v1.23.1...v1.23.2)
#### \[1.23.2] - 2024-07-10
##### Features
- Automatically ignore JWT tokens
</details>
---
### Configuration
📅 **Schedule**: Branch creation - "before 10am on monday" in timezone
Asia/Shanghai, Automerge - At any time (no schedule defined).
🚦 **Automerge**: Enabled.
♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.
🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.
---
- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box
---
This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/oxc-project/oxc).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzEuNCIsInVwZGF0ZWRJblZlciI6IjM3LjQzMS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
```ts
export default class Binding {}
// ^^^^^^^ SymbolFlags::Export
export default function Binding () {}
// ^^^^^^^ SymbolFlags::Export
// No binding, so we should not have SymbolFlags::Export
export default function() {}
export default class {}
```
related: https://github.com/oxc-project/oxc/issues/4142#issuecomment-2219125356
Although we called `enter_node(Class)`, that doesn't mean we're in the `class` scope. It causes our must to visit decorators before `enter_node`.
Let's look at this case. It causes a syntax error if we don't visit decorators before `enter_node`
```js
// This file was procedurally generated from the following sources:
// - src/decorator/decorator-call-expr-identifier-reference-yield.case
// - src/decorator/syntax/valid/cls-expr-decorators-valid-syntax.template
/*---
description: Decorator @ DecoratorCallExpression (Valid syntax for decorator on class expression)
esid: prod-ClassExpression
features: [class, decorators]
flags: [generated, noStrict]
info: |
ClassExpression[Yield, Await] :
DecoratorList[?Yield, ?Await]opt class BindingIdentifier[?Yield, ?Await]opt ClassTail[?Yield, ?Await]
DecoratorList[Yield, Await] :
DecoratorList[?Yield, ?Await]opt Decorator[?Yield, ?Await]
Decorator[Yield, Await] :
@ DecoratorMemberExpression[?Yield, ?Await]
@ DecoratorParenthesizedExpression[?Yield, ?Await]
@ DecoratorCallExpression[?Yield, ?Await]
...
DecoratorCallExpression[Yield, Await] :
DecoratorMemberExpression[?Yield, ?Await] Arguments[?Yield, ?Await]
DecoratorMemberExpression[Yield, Await] :
IdentifierReference[?Yield, ?Await]
DecoratorMemberExpression[?Yield, ?Await] . IdentifierName
DecoratorMemberExpression[?Yield, ?Await] . PrivateIdentifier
IdentifierReference[Yield, Await] :
[~Yield] yield
...
---*/
function decorator() {
return () => {};
}
var yield = decorator;
var C = @yield() class {};
```
Errors:
```shell
× The keyword 'yield' is reserved
╭─[language/statements/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js:45:2]
44 │
45 │ @yield() class C {}
· ─────
╰────
```
The changed code makes more sense. Only if we call `enter_scope` for class, the flags will contain `StrictMode`. Also, we can get the exact `flags` of the `scope` in the `class` at the transformer
For example:
```
class A {
B() {
// Before: flags is `Function`
// After: flags is `Function | StrictMode`
}
}
```
The regression tests will be fixed in follow-up PRs
```ts
class Klass <T> extends Root <R> {}
// ^^^^^ ^^^ ^^^^ ^^^ ^^
// id type_paramter super_class super_type_parameters body
```
I reorder fields according to the order above
The class scope is not defined in the spec. But we need to create a scope for `class` to store `TypeParamters`
