Skip to content

feat(oxc_parser): Port isStartOfDeclaration form tsc#64

Merged
Boshen merged 1 commit into
oxc-project:mainfrom
YangchenYe323:yangchen/is-start-of-declaration
Feb 27, 2023
Merged

feat(oxc_parser): Port isStartOfDeclaration form tsc#64
Boshen merged 1 commit into
oxc-project:mainfrom
YangchenYe323:yangchen/is-start-of-declaration

Conversation

@YangchenYe323

Copy link
Copy Markdown
Contributor

What change is propsoed:

This PR ported the isStartOfDeclaration check to support better parsing of typescript's declarations.

Why the change is necessary:

This could open up opportunities to correctly handle modifiers (abstract, public, declare) of arbitrary order combined with arbitrary declaration items in typescript. For example abstract const a = 1 could be parsed by https://astexplorer.net/ and tsc would reject it at the semantic analysis stage, but not oxc currently can't handle this.

Another benefit is that now constructs likeabstract = 1 can also be correctly parsed by oxc.

Comment thread crates/oxc_parser/src/ts/statement.rs
Comment thread tasks/coverage/typescript.snap

@Boshen Boshen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into parsing TypeScript, it's a really time consuming task.

Comment thread crates/oxc_parser/src/lib.rs Outdated
Comment thread crates/oxc_parser/src/ts/statement.rs
Comment thread crates/oxc_parser/src/ts/statement.rs Outdated
Comment thread tasks/coverage/typescript.snap
@Boshen

Boshen commented Feb 27, 2023

Copy link
Copy Markdown
Member

Can you also fix cargo fmt?

@YangchenYe323 YangchenYe323 force-pushed the yangchen/is-start-of-declaration branch from f2829d6 to 7c6cee7 Compare February 27, 2023 04:06
@YangchenYe323 YangchenYe323 force-pushed the yangchen/is-start-of-declaration branch from 7c6cee7 to baf273e Compare February 27, 2023 04:11
@YangchenYe323 YangchenYe323 requested a review from Boshen February 27, 2023 04:13
@Boshen Boshen merged commit 0bf8f81 into oxc-project:main Feb 27, 2023
@Boshen

Boshen commented Feb 27, 2023

Copy link
Copy Markdown
Member

Thank you!

graphite-app Bot pushed a commit that referenced this pull request Jun 11, 2026
## Summary

- Change `Codegen::wrap` from `FnMut` to `FnOnce`, matching how the helper is used: every closure passed to `wrap` is invoked exactly once.
- Add an assembly comparison note showing the optimized codegen difference before and after the change.

## `Fn`, `FnMut`, and `FnOnce` ?

`Fn`, `FnMut`, and `FnOnce` all describe how a closure may be called:

- `Fn` is the most restrictive for the closure body: it is callable through `&self`, can be called repeatedly, and cannot require mutable or consuming access to captured state.
- `FnMut` is callable through `&mut self`, can be called repeatedly, and may mutate captured state.
- `FnOnce` is callable by value, may consume captured state, and is only guaranteed to be callable once.

The trait relationship goes from most specific to most general call capability:

```rust
Fn: FnMut
FnMut: FnOnce
```

So accepting `FnOnce` is the least restrictive bound for a callback that is only invoked once. It still accepts `Fn` and `FnMut` closures, but it also tells the optimizer that `wrap` does not need a reusable mutable closure object.

## Assembly Impact

`Codegen::wrap` is an inline generic helper, so there is no stable standalone `wrap` assembly symbol in release output. The impact shows up in monomorphized call sites such as `Class::gen`, `Function::gen`, and expression `gen_expr` closures.

Before, several call sites materialized a closure environment on the stack before calling the closure:

```asm
strb    w9, [sp, #15]
add     x9, sp, #15
stp     x0, x9, [sp, #16]
add     x0, sp, #16
bl      <...>::{{closure}}
```

After the `FnOnce` bound, the same shape can pass the one-shot closure state directly through registers:

```asm
and     w1, w2, #0xfffffffd
mov     x2, x19
bl      <...>::{{closure}}
```

Some wrapper frames also shrink. For example, representative `Class::gen` / `Function::gen` paths go from a `64` byte frame to a `48` byte frame:

```asm
- sub     sp, sp, #64
- stp     x20, x19, [sp, #32]
- stp     x29, x30, [sp, #48]
+ sub     sp, sp, #48
+ stp     x20, x19, [sp, #16]
+ stp     x29, x30, [sp, #32]
```

Several restored-frame return paths also become tail calls:

```asm
ldp     x29, x30, [sp, #32]
ldp     x20, x19, [sp, #16]
add     sp, sp, #48
b       <closure or push_slow target>
```

The assembly diff also contains expected local label renumbering noise, such as switch-table suffixes changing from `.318` to `.330`; those are not behavior changes.
camc314 added a commit that referenced this pull request Jul 3, 2026
## Summary

- Change `Codegen::wrap` from `FnMut` to `FnOnce`, matching how the helper is used: every closure passed to `wrap` is invoked exactly once.
- Add an assembly comparison note showing the optimized codegen difference before and after the change.

## `Fn`, `FnMut`, and `FnOnce` ?

`Fn`, `FnMut`, and `FnOnce` all describe how a closure may be called:

- `Fn` is the most restrictive for the closure body: it is callable through `&self`, can be called repeatedly, and cannot require mutable or consuming access to captured state.
- `FnMut` is callable through `&mut self`, can be called repeatedly, and may mutate captured state.
- `FnOnce` is callable by value, may consume captured state, and is only guaranteed to be callable once.

The trait relationship goes from most specific to most general call capability:

```rust
Fn: FnMut
FnMut: FnOnce
```

So accepting `FnOnce` is the least restrictive bound for a callback that is only invoked once. It still accepts `Fn` and `FnMut` closures, but it also tells the optimizer that `wrap` does not need a reusable mutable closure object.

## Assembly Impact

`Codegen::wrap` is an inline generic helper, so there is no stable standalone `wrap` assembly symbol in release output. The impact shows up in monomorphized call sites such as `Class::gen`, `Function::gen`, and expression `gen_expr` closures.

Before, several call sites materialized a closure environment on the stack before calling the closure:

```asm
strb    w9, [sp, #15]
add     x9, sp, #15
stp     x0, x9, [sp, #16]
add     x0, sp, #16
bl      <...>::{{closure}}
```

After the `FnOnce` bound, the same shape can pass the one-shot closure state directly through registers:

```asm
and     w1, w2, #0xfffffffd
mov     x2, x19
bl      <...>::{{closure}}
```

Some wrapper frames also shrink. For example, representative `Class::gen` / `Function::gen` paths go from a `64` byte frame to a `48` byte frame:

```asm
- sub     sp, sp, #64
- stp     x20, x19, [sp, #32]
- stp     x29, x30, [sp, #48]
+ sub     sp, sp, #48
+ stp     x20, x19, [sp, #16]
+ stp     x29, x30, [sp, #32]
```

Several restored-frame return paths also become tail calls:

```asm
ldp     x29, x30, [sp, #32]
ldp     x20, x19, [sp, #16]
add     sp, sp, #48
b       <closure or push_slow target>
```

The assembly diff also contains expected local label renumbering noise, such as switch-table suffixes changing from `.318` to `.330`; those are not behavior changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants