Skip to content

Align visitation order in Traverse with Visit and VisitMut #3392

@overlookmotel

Description

@overlookmotel

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:

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_program
  • enter_directive + exit_directive (for each directive)
  • enter_hashbang + exit_hashbang
  • enter_statements + exit_statements
  • exit_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 Visit and VisitMut which does visitation in AST node field order (like Traverse does).
  • Diff current Visit vs codegen-ed Visit.
  • Where they don't match, change the field order in AST types, and run codegen again.
  • Keep doing that until the codegen-ed Visit and current hand-written Visit match exactly.
  • Throw away the old hand-written Visit and VisitMut and 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

A-transformerArea - Transformer / TranspilerC-cleanupCategory - technical debt or refactoring. Solution not expected to change behavior

Type

No type

Priority

None yet

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions