apache / apache/datafusion-sqlparser-rs
Question: why is the Visitor trait limited to statements, relations & expressions?
- Dominant language
- Rust
- Stars
- 3.5k
- Forks
- 772
- Avg merge
- 4d 9h
- Merged PRs (30d)
- 17
Description
What is the reason for that particular design decision versus providing a more general `Visitor` implementation?
Two options for a generalised Visitor trait come to mind:
1. expose pre + post trait method variants for every AST node type, or
2. expose only two trait methods (`pre_visit` + `post_visit`) with signatures like `fn pre_visit(&mut self, node: &AstNode) -> ControlFlow` - where `AstNode` is an enum with a wrapper variant for every AST node type found in `src/ast/mod.rs` and can be `match`ed against.
Would the maintainers be interested in a PR that implements one of the above two approaches?
My preference would be for option 2 because it would not break the trait when node types are added/removed.
Suggested approach:
1. Define a new `RawVisitor` trait (and `RawVisitorMut` trait) like this:
```rust
pub trait RawVisitor {
type Break;
fn pre_visit(&mut self, node: &AstNode) -> ControlFlow;
fn post_visit(&mut self, node: &AstNode) -> ControlFlow;
}
```
2. Define an adapter type (`RawVisitorAdapter` ?) that accepts a `V: Visitor` generic argument and implements `RawVisitor` & `RawVisitorMut`, which calls the appropriate method on `V` (or none at all)
```rust
struct RawVisitorAdapter(v);
impl RawVisitor for RawVisitorAdapter {
type Break = V::Break;
fn pre_visit(&mut self, node: &AstNode) -> ControlFlow {
match node {
AstNode(Statement) => self.0.pre_visit_statement(...),
// etc
}
}
fn post_visit(&mut self, node: &AstNode) -> ControlFlow;
}
```
3. Change the `Visit` derivation macros to generate code in terms of `RawVisitor` & `RawVisitorMut` instead of `Visitor`, like this:
```rust
pub trait Visit {
fn visit_raw(&self, visitor: &mut V) -> ControlFlow;
// This has an identical signature to the existing trait, but has a default implementation
fn visit(&self, visitor: &mut V) -> ControlFlow {
self.visit_raw(RawVisitorAdapter::new(visitor))
}
}
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Review the existing Visitor traits and Visit derivation macros, then inspect the AST node definitions in src/ast/mod.rs. First confirm with maintainers whether the RawVisitor adapter design or per-node hooks is wanted; done means implementing the agreed generalized visitor API while preserving existing Visitor behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100