Skip to content

Refactor: Replace dynamic_cast chain with Visitor pattern #141

Description

@clank-hayen

Problem

Multiple files use long chains of `dynamic_cast` for AST node dispatch:

  1. codegen/emit_expression.cpp (lines 22-100+):

    if (auto* lit = dynamic_cast<const StringLiteral*>(&node)) { ... }
    if (auto* lit = dynamic_cast<const NumberLiteral*>(&node)) { ... }
    // ... 20+ more type checks
  2. codegen/emit_statement.cpp: Similar pattern for statements

  3. typechecker/check_expression.cpp: Expression type inference

This approach has issues:

  • Multiple virtual calls per node (one per failed dynamic_cast)
  • Easy to miss a node type (no compile-time checking)
  • Long if-else chains are hard to maintain

Suggested Solution

Implement the Visitor pattern:

// In ast.hpp
struct ASTVisitor {
    virtual void visit(const StringLiteral&) = 0;
    virtual void visit(const NumberLiteral&) = 0;
    virtual void visit(const BinaryExpr&) = 0;
    // ... one per node type
};

struct ASTNode {
    virtual void accept(ASTVisitor& v) const = 0;
    // ...
};

// Each node implements:
void StringLiteral::accept(ASTVisitor& v) const { v.visit(*this); }

Alternatively, use a variant-based approach with `std::visit`.

Trade-offs

Pros:

  • Compile-time enforcement that all cases are handled
  • Single virtual call per node
  • Cleaner code organization

Cons:

  • More boilerplate in AST definitions
  • Slightly more complex to add new node types

Priority

Medium - the current approach works, but as the AST grows, this becomes increasingly important.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions