feat(ast): add AstNode trait and provide access to AstNodeId

[rzvxa]

closes #2818

[rzvxa]

• experiment(ast): rework enum of structures #2877
• feat(ast): add AstNode trait and provide access to AstNodeId #2861 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #2861 will degrade performances by 3.05%

_{Comparing 03-30-feat_ast_add_astnode_trait_and_its_derive_macro (4ae5c5e) with main (7034bcc)}

Summary

❌ 2 regressions
✅ 34 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	03-30-feat_ast_add_astnode_trait_and_its_derive_macro	Change
❌	parser_napi[RadixUIAdoptionSection.jsx]	230.1 µs	237.3 µs	-3.03%
❌	semantic[checker.ts]	541 ms	558 ms	-3.05%

[rzvxa]

@Boshen I like the idea of keeping the raw ast node IDs private to the crate, But it would mean that we have to either add new functions for a bunch of types or add missing types to ast_builder so we can use that in all cases instead of constructing with Type { ... } syntax. With ast_node_id private only enum types can be initialized directly.

Should I go for it or make the field public? The public field is a bit less encapsulated and results in a more verbose construction since you always need to set the field to default but reduces the dependency on ast builder, Do we want our types to even be initialized without a builder? Or do we desire to keep all allocations in a singular point? The ladder by default means that moving constructions to the builder is a plus but I can't say the same thing for the first one.

[rzvxa]

If we move all allocations to the builder we might want to make them inline as well to reduce the jumps while constructing.

[rzvxa]

I'll make it public for now so it won't stop my progress, But it is up for debate if anyone wants to pitch in on how we would approach it. I can address it in an upcoming PR.

[overlookmotel]

In my view AstNodeId should be a non-zero type e.g. NonZeroUsize. Then AstNodeIdContainer will be half the size.

std::mem::size_of::<Cell<Option<usize>>>() == 16
std::mem::size_of::<Cell<Option<NonZeroUsize>>>() == 8

This would likely reduce the performance reduction visible on this PR at present.

You'd just need to initialize AstNodes::nodes and AstNodes::parent_ids with dummy entries to occupy index 0, and enforce that that dummy entry can never be popped off again (maybe wrap both Vecs to make them append-only).

Also, in my view AstNodeId could be a u32 (by which I mean NonZeroU32) for the reasons mentioned in #2876 (comment). But that's a separate argument from what I'm saying here.

[overlookmotel]

Let's keep this branch live, even though PR is closed. We may want to come back to this later.