Skip to content

HIR/THIR visitors should deconstruct all values #141849

Closed
@nnethercote

Description

@nnethercote

The AST visitor (in compiler/rustc_ast/src/visit.rs) uses struct/enum deconstruction to access fields in its walk_* methods, e.g.:

pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V::Result {
    let Expr { id, kind, span, attrs, tokens: _ } = expression; 
    walk_list!(visitor, visit_attribute, attrs);
    match kind {
        ...
    }
    ...
}

This is good, because it's impossible to add a new field to an AST type and fail to update the appropriate visitor -- you'll get a compile error. (I swear there was a comment about this, but I can't find it right now.)

In contrast, the HIR visitor (in compiler/rustc_hir/src/intravisit.rs) uses field access:

pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) -> V::Result {
    try_visit!(visitor.visit_id(expression.hir_id));
    match expression.kind {
        ...
    }
    ...
}

It would be good to change the HIR visitor to use deconstruction everywhere. This would identify any fields that are currently erroneously unvisited, and prevent future occurrences.

Likewise for the THIR visitor (in compiler/rustc_middle/src/thir/visit.rs) which is a lot smaller and simpler.

This would be a good task for a new contributor, maybe even a good first bug.

Metadata

Metadata

Assignees

Labels

C-cleanupCategory: PRs that clean code up or issues documenting cleanup.E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions