-
-
Notifications
You must be signed in to change notification settings - Fork 933
Description
Traverse is codegen-ed from the AST node type definitions. The codegen generates a walker which visits children of a node in the order which they're defined in the AST types. e.g. for:
oxc/crates/oxc_ast/src/ast/js.rs
Lines 52 to 60 in 8a1db67
| pub struct Program<'a> { | |
| #[cfg_attr(feature = "serialize", serde(flatten))] | |
| pub span: Span, | |
| pub source_type: SourceType, | |
| pub directives: Vec<'a, Directive<'a>>, | |
| pub hashbang: Option<Hashbang<'a>>, | |
| pub body: Vec<'a, Statement<'a>>, | |
| pub scope_id: Cell<Option<ScopeId>>, | |
| } |
Visitors are called in order matching the above:
enter_programenter_directive+exit_directive(for each directive)enter_hashbang+exit_hashbangenter_statements+exit_statementsexit_program
On the other hand, Visit and VisitMut were written by hand. It's likely that for some types the visitation order is different from the types' field order. So it will be different from Traverse's visitation order.
This mismatch is going to cause problems at some point as contributors who are used to Visit/VisitMut will assume that Traverse behaves the same, and be confused when it doesn't, or unwittingly introduce bugs due to incorrect assumptions.
I think best way to tackle this is:
- Write a codegen for
VisitandVisitMutwhich does visitation in AST node field order (likeTraversedoes). - Diff current
Visitvs codegen-edVisit. - Where they don't match, change the field order in AST types, and run codegen again.
- Keep doing that until the codegen-ed
Visitand current hand-writtenVisitmatch exactly. - Throw away the old hand-written
VisitandVisitMutand use the codegen from here on.
Once we switch to codegen, any changes to AST types will automatically update Visit + VisitMut to match.
In addition: We know there are also some discrepancies with Visit/VisitMut not visiting some child nodes. Likely in some cases this is a mistake (inevitable when writing this much code by hand), but in other cases it's intentional (like not visiting some TS nodes where it's pointless to do so).
The process above will reveal all such discrepancies, and then we can decide which are mistakes and which are intentional, and we can then adapt Visit, VisitMut and Traverse to make them all consistent and all reflecting what we decide is the "correct" visitation policy.
Metadata
Metadata
Assignees
Labels
Type
Fields
Give feedbackPriority