part of #3213
We should only have one diagnostic struct instead 353 copies of them, so we don't end up choking LLVM with 50k lines of the same code due to monomorphization.
If the proposed approach is good, then I'll start writing a codemod to turn all the existing structs to plain functions.
---
Background:
Using `--timings`, we see `oxc_linter` is slow on codegen (the purple part).
The crate currently contains 353 miette errors. [cargo-llvm-lines](https://github.com/dtolnay/cargo-llvm-lines) displays
```
cargo llvm-lines -p oxc_linter --lib --release
Lines Copies Function name
----- ------ -------------
830350 33438 (TOTAL)
29252 (3.5%, 3.5%) 808 (2.4%, 2.4%) <alloc::boxed::Box<T,A> as core::ops::drop::Drop>::drop
23298 (2.8%, 6.3%) 353 (1.1%, 3.5%) miette::eyreish::error::object_downcast
19062 (2.3%, 8.6%) 706 (2.1%, 5.6%) core::error::Error::type_id
12610 (1.5%, 10.1%) 65 (0.2%, 5.8%) alloc::raw_vec::RawVec<T,A>::grow_amortized
12002 (1.4%, 11.6%) 706 (2.1%, 7.9%) miette::eyreish::ptr::Own<T>::boxed
9215 (1.1%, 12.7%) 115 (0.3%, 8.2%) core::iter::traits::iterator::Iterator::try_fold
9150 (1.1%, 13.8%) 1 (0.0%, 8.2%) oxc_linter::rules::RuleEnum::read_json
8825 (1.1%, 14.9%) 353 (1.1%, 9.3%) <miette::eyreish::error::ErrorImpl<E> as core::error::Error>::source
8822 (1.1%, 15.9%) 353 (1.1%, 10.3%) miette::eyreish::error::<impl miette::eyreish::Report>::construct
8119 (1.0%, 16.9%) 353 (1.1%, 11.4%) miette::eyreish::error::object_ref
8119 (1.0%, 17.9%) 353 (1.1%, 12.5%) miette::eyreish::error::object_ref_stderr
7413 (0.9%, 18.8%) 353 (1.1%, 13.5%) <miette::eyreish::error::ErrorImpl<E> as core::fmt::Display>::fmt
7413 (0.9%, 19.7%) 353 (1.1%, 14.6%) miette::eyreish::ptr::Own<T>::new
6669 (0.8%, 20.5%) 39 (0.1%, 14.7%) alloc::raw_vec::RawVec<T,A>::try_allocate_in
6173 (0.7%, 21.2%) 353 (1.1%, 15.7%) miette::eyreish::error::<impl miette::eyreish::Report>::from_std
6027 (0.7%, 21.9%) 70 (0.2%, 16.0%) <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
6001 (0.7%, 22.7%) 353 (1.1%, 17.0%) miette::eyreish::error::object_drop
6001 (0.7%, 23.4%) 353 (1.1%, 18.1%) miette::eyreish::error::object_drop_front
5648 (0.7%, 24.1%) 353 (1.1%, 19.1%) <miette::eyreish::error::ErrorImpl<E> as core::fmt::Debug>::fmt
```
It's totalling more than 50k llvm lines, and is putting pressure on rustc codegen (the purple part on `oxc_linter` in the image above.
---
It's pretty obvious by looking at https://github.com/zkat/miette/blob/main/src/eyreish/error.rs, the generics can expand out to lots of code.
This PR builds on #3201 to further speed up the benchmarks and reduce CI
time.
* Build and run each benchmark as separate job (like before).
* But now each bench is only built with the dependencies it needs.
* For linter benchmarks, build benchmark in 1 job (like #3201 does).
* Run each linter fixture in a separate job as they're slow.
This reduces total time to complete benchmarks from between 6m-7m to
~4m40s.
All the individual jobs complete in under 1m30s, except for building
linter benchmark which takes 2m30s. So there won't be the problem of
blocking the CI queue that there was before.
NB: I did try this before, and didn't see a benefit. But I realized
today what I was doing wrong - it only works once the caches are
populated by a previous run on main branch.
So the CI times in this PR won't look good, but once it's merged to
main, it will take effect. Here it is running on main branch of my fork:
https://github.com/overlookmotel/oxc/actions/runs/9030511348
I also added a step to delete the temp artefacts which aren't needed
once the run has completed.
Sliced off from #3152.
Re-implement `transform-react-display-name` using bottom-up lookup
provided by `Traverse` trait.
This fixes the 1 remaining failing test case for this plugin (see
#2937).
`Traverse` is not complete yet (see #3182), so this is also not ready to
merge yet.
It seems like we need to rebuild the scopes and symbols while
traversing. We can't utilize the scopes and symbols built by semantic
because they are immutable.
The remaining test cases will perform better with a scope
implementation, and while we can implement them without the scope, it
still requires us to do what the scope did.
---------
Co-authored-by: Boshen <boshenc@gmail.com>
OK, this is a big one...
I have done this as part of work on Traversable AST, but I believe it
has wider benefits, so thought better to spin it off into its own PR.
## What this PR does
This PR squashes all nested AST enum types (#2685).
e.g.: Previously:
```rs
pub enum Statement<'a> {
BlockStatement(Box<'a, BlockStatement<'a>>),
/* ...other Statement variants... */
Declaration(Declaration<'a>),
}
pub enum Declaration<'a> {
VariableDeclaration(Box<'a, VariableDeclaration<'a>>),
/* ...other Declaration variants... */
}
```
After this PR:
```rs
#[repr(C, u8)]
pub enum Statement<'a> {
BlockStatement(Box<'a, BlockStatement<'a>>) = 0,
/* ...other Statement variants... */
VariableDeclaration(Box<'a, VariableDeclaration<'a>>) = 32,
/* ...other Declaration variants... */
}
#[repr(C, u8)]
pub enum Declaration<'a> {
VariableDeclaration(Box<'a, VariableDeclaration<'a>>) = 32,
/* ...other Declaration variants... */
}
```
All `Declaration`'s variants are combined into `Statement`, but
`Declaration` type still exists.
As both types are `#[repr(C, u8)]`, and the discriminants are aligned, a
`Declaration` can be transmuted to a `Statement` at zero cost.
This is the same thing as #2847, but here applied to *all* nested enums
in the AST, and with improved helper methods.
No enums increase in size, and a few get smaller. Indirection is reduced
for some types (this removes multiple levels of boxing).
## Why?
1. It is a prerequisite for Traversable AST (#2987).
2. It would help a lot with AST Transfer (#2409) - it solves the only
remaining blocker for this.
3. It is a step closer to making the whole AST `#[repr(C)]`.
## Why is it a good thing for the AST to be `#[repr(C)]`?
Oxc's direction appears to be increasingly to build up control over the
fundamental primitives we use, in order to unlock performance and
features. We have our own allocator, our own custom implementations for
`Box` and `Vec`, our own `IndexVec` (TBC). The AST is the central
building block of Oxc, and taking control of its memory layout feels
like a step in this same direction.
Oxc has a major advantage over other similar libraries in that it keeps
all the AST data in an arena. This opens the door to treating the AST
either as Rust types or as *pure data* (just bytes). That data can be
moved around and manipulated beyond what Rust natively allows.
However, to enable that, the types need to be well-specified, with
completely stable layouts. `#[repr(C)]` is the only tool Rust provides
to do this.
Once the types are `#[repr(C)]`, various features become possible:
1. Cheap transfer of the AST across boundaries without ser/deser - the
property used by AST Transfer.
2. Having multiple versions of the AST (standard, read-only,
traversable), and these AST representations can be converted to one
other at zero cost via transmute - the property used by Traversable AST
scheme.
3. Caching AST data on disk (#3079) or transferring across network.
4. Stuff we haven't thought of yet!
Allowing the AST to be treated as pure data will likely unlock other
"next level" features further down the track (caching for "edge
bundling" comes to mind).
## The problem with `#[repr(C)]`
It's not *required* to squash nested enums to make the AST `#[repr(C)]`.
But the problem with `#[repr(C)]` is that it disables some compiler
optimizations. Without `#[repr(C)]`, the compiler squashes enums itself
in some cases (which is how `Statement` is currently 16 bytes). But
making the types `#[repr(C)]` as they are currently disables this
optimization.
So this PR essentially makes explicit what the compiler is already doing
- and in fact goes a bit further with the optimization than the compiler
is able to, in squashing 3 or 4 layers of nested enums (the compiler
only does up to 2 layers).
## Implementation
One enum "inheriting" variants from another is implemented with
`inherit_variants!` macro.
```rs
inherit_variants! {
#[repr(C, u8)]
pub enum Statement<'a> {
BlockStatement(Box<'a, BlockStatement<'a>>),
/* ...other Statement variants... */
// `Declaration` variants added here by `inherit_variants!` macro
@inherit Declaration
// `ModuleDeclaration` variants added here by `inherit_variants!` macro
@inherit ModuleDeclaration
}
}
```
The macro is *fairly* lightweight, and I think the above is quite easy
to understand. No proc macros.
The macro also implements utility methods for converting between enums
e.g. `Statement::as_declaration`. These methods are all zero-cost
(essentially transmutes).
New patterns for dealing with nested enums are introduced:
Creation:
```rs
// Old
let stmt = Statement::Declaration(Declaration::VariableDeclaration(var_decl));
// New
let stmt = Statement::VariableDeclaration(var_decl);
```
Conversion:
```rs
// Old
let stmt = Statement::Declaration(decl);
// New
let stmt = Statement::from(decl);
```
Testing:
```rs
// Old
if matches!(stmt, Statement::Declaration(_)) { }
if matches!(stmt, Statement::ModuleDeclaration(m) if m.is_import()) { }
// New
if stmt.is_declaration() { }
if matches!(stmt, Statement::ImportDeclaration(_)) { }
```
Branching:
```rs
// Old
if let Statement::Declaration(decl) = &stmt { decl.do_stuff() };
// New
if let Some(decl) = stmt.as_declaration() { decl.do_stuff() };
```
Matching:
```rs
// Old
match stmt {
Statement::Declaration(decl) => visitor.visit(decl),
}
// New (exhaustive match)
match stmt {
match_declaration!(Statement) => visitor.visit(stmt.to_declaration()),
}
// New (alternative)
match stmt {
_ if stmt.is_declaration() => visitor.visit(stmt.to_declaration()),
}
```
New syntax has pluses and minuses vs the old. `match` syntax is worse,
but when working with a deeply nested enum, the code is much nicer -
it's shorter and easier to read.
This PR removes 200 lines from the linter with changes like this:
https://github.com/oxc-project/oxc/pull/3115/files#diff-dc417ff57352da6727a760ec6dee22de6816f8231fb69dbef1bf05d478699103L92-R95
```diff
- let AssignmentTarget::SimpleAssignmentTarget(simple_assignment_target) =
- &assignment_expr.left
- else {
- return;
- };
- let SimpleAssignmentTarget::AssignmentTargetIdentifier(ident) =
- simple_assignment_target
+ let AssignmentTarget::AssignmentTargetIdentifier(ident) = &assignment_expr.left
else {
return;
};
```
Re-use allocator in parser + lexer benchmarks.
I believe this is the recommended usage when parsing a bunch of files -
to re-use one allocator rather than create a fresh one for each run, so
it makes sense to me that this is what the benchmark should measure.
Doesn't show much difference on CodSpeed because it only runs the
benchmark once, and it treats allocations as free anyway. But I imagine
the difference may show up a bit more in a standard criterion benchmark.
`codegen` relies on the modifier to decide whether to print or not. For
example, if declare is present, nothing will be printed if typescript is
not enabled.
We may need to solve this problem in a way that doesn't rely on
modifers.
