Skip to content

Detect situations where a visitor implementation is skipped by accidentally directly calling the corresponding walk function #129859

Open
@compiler-errors

Description

@compiler-errors

For the AST (but also other visitors in the compiler), visitor functions typically come in two flavors:

  • visit_X - Overrideable part of the trait invocation. If you override it, you should call walk_X if you want to recurse or do visit_Y calls on the sub-components (for some other Y) you want to walk into, skipping the rest.
  • walk_X - Non-overrideable, free function which actually recurses on the structure by calling visit_Y on all of the sub-components (for some other Y) of the thing. Sometimes this is called super_visit_X.

It is typically a bug to call walk_Y on a sub-component within a visit_X invocation. For example, see:

#129858

Which fixed a bug where we called walk_expr on the async closure's body when visiting the async closure in visit_fn. This should've been a visit_expr, since visit_expr had meaningful side-effects (collecting macro invocations).

We should have some sort of lint to prevent this pattern outside of regular visit_X -> walk_X situation, and compiler devs who need to do something special can do #[allow(whatever_lint_name_for_sketchy_visitor_pattern)] to justify that what they're doing is okay. We already have some comments that document when this happens (I think, I feel like I've seen them around), but it can be done by accident which causes bugs!

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.C-cleanupCategory: PRs that clean code up or issues documenting cleanup.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions