Skip to content

Commit 4b7a1c7

Browse files
committed
Implement visitor pattern infrastructure (Phase 1 & 2)
This commit implements Phases 1 and 2 of the refactoring proposal: - Phase 1: Visitor pattern infrastructure - Phase 2: Specialized visitors (SemanticAnalyzer and DebugPrinter) **Phase 1: Infrastructure** - Added Visitor interface with methods for all AST node types - Added Accept() methods to all AST nodes - Created BaseVisitor with default traversal implementations - Added comprehensive unit tests for visitor pattern **Phase 2: Specialized Visitors** - Implemented SemanticAnalyzer for validating: - Variable declarations and usage - Undefined variable detection - Proper scoping rules - Channel usage validation - Implemented DebugPrinter for debugging: - Pretty-printed AST representation - Hierarchical tree structure visualization - Useful for development and debugging **Files Added:** - pkg/ast/visitor.go - Visitor interface definition - pkg/ast/base_visitor.go - BaseVisitor implementation - pkg/ast/visitor_test.go - Visitor infrastructure tests - pkg/visitors/semantic_analyzer.go - SemanticAnalyzer implementation - pkg/visitors/debug_printer.go - DebugPrinter implementation - pkg/visitors/visitors_test.go - Specialized visitor tests **Files Modified:** - pkg/ast/ast.go - Added Accept() methods to all AST nodes **Benefits:** - ✅ Clean separation of concerns - ✅ Modular, testable components - ✅ Easy to add new analysis/transformation passes - ✅ Foundation for future refactoring (Phases 3-5) **Tests:** All tests passing: - pkg/ast/visitor_test.go: 5/5 tests pass - pkg/visitors/visitors_test.go: 7/7 tests pass **Next Steps:** Phase 3 will migrate the existing Generator to use the visitor pattern while maintaining identical output (non-breaking change). Related to refactoring proposal docs: - docs/REFACTORING_PROPOSAL.md - docs/visitor-pattern-proposal.md - docs/implementation-plan.md
1 parent 5e60532 commit 4b7a1c7

11 files changed

Lines changed: 4167 additions & 0 deletions

docs/REFACTORING_PROPOSAL.md

Lines changed: 352 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,352 @@
1+
# Guix Compiler Refactoring Proposal
2+
3+
## Executive Summary
4+
5+
This proposal outlines a comprehensive refactoring of the Guix compiler to:
6+
1. **Eliminate grammar ambiguity** preventing arbitrary function calls in .gx files
7+
2. **Introduce visitor pattern** for better code organization and extensibility
8+
3. **Enable new features** while maintaining backward compatibility
9+
10+
## Problem Statement
11+
12+
### Current Issue
13+
The parser cannot distinguish between:
14+
- Function calls: `log(...)`
15+
- Method calls: `console.Call(...)`
16+
- Field assignments: `state.Display = value`
17+
- UI elements: `Div(...) { ... }`
18+
19+
All start with the same pattern (`Ident ("." Ident)* ("(" ... ")")?`), causing parse errors.
20+
21+
### Impact
22+
- ❌ Cannot add logging to helper functions
23+
- ❌ Grammar is brittle and hard to extend
24+
- ❌ Code generation is tightly coupled to AST structure
25+
- ❌ Difficult to add preprocessing/validation passes
26+
27+
## Proposed Solution
28+
29+
### Two-Pronged Approach
30+
31+
#### 1. Grammar Restructuring
32+
**Principle**: Parse liberally, disambiguate later
33+
34+
**Key Change**: Introduce `ExpressionStatement` that unifies expressions and assignments:
35+
36+
```go
37+
type ExpressionStatement struct {
38+
Left *CallOrSelect `@@`
39+
Op string `(@("=" | ":=" | ...))?` // Optional!
40+
Right *Expr `(@@)?` // Optional!
41+
}
42+
```
43+
44+
**Disambiguation**:
45+
- If `Op` is empty → Expression statement
46+
- If `Op` is present → Assignment statement
47+
48+
**Benefits**:
49+
- ✅ No parser ambiguity
50+
- ✅ Simpler grammar
51+
- ✅ Supports all statement types
52+
53+
#### 2. Visitor Pattern
54+
**Principle**: Separate concerns into composable passes
55+
56+
**Architecture**:
57+
```
58+
Parser → AST
59+
60+
Semantic Analyzer (Visitor) → Validated AST
61+
62+
Statement Validator (Visitor) → Transformed AST
63+
64+
Code Generator (Visitor) → Go AST → Code
65+
```
66+
67+
**Benefits**:
68+
- ✅ Modular, testable components
69+
- ✅ Easy to add new passes (optimization, etc.)
70+
- ✅ Clean separation of concerns
71+
- ✅ Maintainable codebase
72+
73+
## Detailed Proposals
74+
75+
### 1. Grammar Refactoring
76+
See: [`grammar-refactor-proposal.md`](./grammar-refactor-proposal.md)
77+
78+
**Key Points**:
79+
- Unified `ExpressionStatement` type
80+
- Self-disambiguating via optional operator
81+
- Backward compatible during transition
82+
- Examples of parsing different statement types
83+
84+
### 2. Visitor Pattern Architecture
85+
See: [`visitor-pattern-proposal.md`](./visitor-pattern-proposal.md)
86+
87+
**Key Points**:
88+
- Visitor interface definition
89+
- BaseVisitor for default traversal
90+
- Specialized visitors (SemanticAnalyzer, CodeGenerator)
91+
- Migration strategy
92+
- Examples of extending with new visitors
93+
94+
### 3. Implementation Plan
95+
See: [`implementation-plan.md`](./implementation-plan.md)
96+
97+
**Key Points**:
98+
- 5 phases over 10-15 days
99+
- Non-breaking initial phases
100+
- Comprehensive testing strategy
101+
- Risk mitigation
102+
- Success criteria
103+
104+
## Benefits
105+
106+
### Immediate
107+
-**Function calls work**: `log(fmt.Sprintf(...))`
108+
-**Method calls work**: `console.Call("log", ...)`
109+
-**Field assignments work**: `state.Display = value`
110+
111+
### Long-term
112+
-**Extensibility**: Easy to add new language features
113+
-**Maintainability**: Cleaner, modular codebase
114+
-**Testability**: Each visitor tested independently
115+
-**Debugging**: Debug printer visitor for AST inspection
116+
-**Optimization**: Easy to add optimization passes
117+
-**Validation**: Separate semantic analysis pass
118+
-**Documentation**: Self-documenting visitor architecture
119+
120+
## Examples
121+
122+
### Before (Doesn't Work)
123+
```go
124+
func handleNumber(...) {
125+
log(fmt.Sprintf("Debug: %v", value)) // ❌ Parse error
126+
state.Display = digit // ❌ Ambiguity error
127+
}
128+
```
129+
130+
### After (Works!)
131+
```go
132+
func handleNumber(...) {
133+
log(fmt.Sprintf("Debug: %v", value)) // ✅ Expression statement
134+
state.Display = digit // ✅ Assignment
135+
state.Counter = state.Counter + 1 // ✅ Complex assignment
136+
}
137+
```
138+
139+
## Migration Path
140+
141+
### Phase 1: Infrastructure (Non-Breaking)
142+
- Add visitor interfaces
143+
- Add Accept() methods to AST
144+
- Create BaseVisitor
145+
- **No changes to existing code**
146+
147+
### Phase 2: Visitors (Non-Breaking)
148+
- Implement SemanticAnalyzer
149+
- Implement DebugPrinter
150+
- Test independently
151+
- **No changes to codegen yet**
152+
153+
### Phase 3: Migrate Generator (Non-Breaking)
154+
- Make Generator implement Visitor
155+
- Refactor using Accept() calls
156+
- **Output remains identical**
157+
158+
### Phase 4: Grammar Update (Breaking)
159+
- Introduce ExpressionStatement
160+
- Update parser
161+
- Update generator
162+
- **New functionality enabled**
163+
164+
### Phase 5: Cleanup
165+
- Remove old code
166+
- Update documentation
167+
- Ship!
168+
169+
## Testing Strategy
170+
171+
### Unit Tests
172+
- Each visitor tested independently
173+
- AST Accept() methods verified
174+
- Grammar parsing edge cases
175+
176+
### Integration Tests
177+
- Regenerate all examples
178+
- Compare with previous output
179+
- Test new function call support
180+
181+
### Regression Tests
182+
- All existing tests must pass
183+
- Backward compatibility verified
184+
185+
## Timeline
186+
187+
- **Phase 1**: 2-3 days
188+
- **Phase 2**: 2-3 days
189+
- **Phase 3**: 3-4 days
190+
- **Phase 4**: 2-3 days
191+
- **Phase 5**: 1-2 days
192+
193+
**Total**: 10-15 days
194+
195+
## Success Criteria
196+
197+
- ✅ Function calls work in helper functions
198+
- ✅ Method calls work (console.Call, etc.)
199+
- ✅ Field assignments work (state.field = value)
200+
- ✅ All existing examples regenerate correctly
201+
- ✅ All tests pass
202+
- ✅ Documentation complete
203+
- ✅ Code is more maintainable than before
204+
205+
## Risks & Mitigation
206+
207+
### Risk: Breaking Changes
208+
**Mitigation**: Gradual migration, keep old code until proven
209+
210+
### Risk: Performance
211+
**Mitigation**: Benchmark before/after, optimize if needed
212+
213+
### Risk: Complexity
214+
**Mitigation**: Comprehensive docs, clear examples, code reviews
215+
216+
## Comparison with Current Approach
217+
218+
### Current (Type Switches)
219+
```go
220+
func generateStatement(stmt *Statement) ast.Stmt {
221+
if stmt.Assignment != nil {
222+
// 50 lines of assignment logic
223+
}
224+
if stmt.Expr != nil {
225+
// 30 lines of expr logic
226+
}
227+
if stmt.If != nil {
228+
// 40 lines of if logic
229+
}
230+
// ... 10 more type checks
231+
}
232+
```
233+
234+
**Issues**:
235+
- Monolithic function
236+
- Hard to test
237+
- Tightly coupled
238+
- Difficult to extend
239+
240+
### Proposed (Visitor Pattern)
241+
```go
242+
func (g *Generator) VisitExpressionStatement(n *ExprStmt) interface{} {
243+
if n.Op != "" {
244+
return g.generateAssignment(n)
245+
}
246+
return g.generateExpression(n)
247+
}
248+
249+
func (g *Generator) VisitIfStmt(n *IfStmt) interface{} {
250+
return g.generateIf(n)
251+
}
252+
```
253+
254+
**Benefits**:
255+
- ✅ Each method focused on one thing
256+
- ✅ Easy to test
257+
- ✅ Loosely coupled
258+
- ✅ Easy to extend
259+
260+
## Future Possibilities
261+
262+
With visitor pattern in place, we can easily add:
263+
264+
### Optimization Pass
265+
```go
266+
type Optimizer struct {
267+
BaseVisitor
268+
}
269+
270+
func (o *Optimizer) VisitExpr(n *Expr) interface{} {
271+
// Constant folding
272+
// Dead code elimination
273+
// etc.
274+
}
275+
```
276+
277+
### Type Inference
278+
```go
279+
type TypeInferencer struct {
280+
BaseVisitor
281+
types map[*ast.Node]Type
282+
}
283+
```
284+
285+
### Documentation Generator
286+
```go
287+
type DocGenerator struct {
288+
BaseVisitor
289+
}
290+
291+
func (d *DocGenerator) VisitComponent(n *Component) interface{} {
292+
// Generate markdown docs
293+
}
294+
```
295+
296+
### Linter
297+
```go
298+
type Linter struct {
299+
BaseVisitor
300+
warnings []Warning
301+
}
302+
```
303+
304+
## Conclusion
305+
306+
This refactoring provides:
307+
1. **Immediate value**: Solves function call ambiguity
308+
2. **Long-term value**: Better architecture for future growth
309+
3. **Low risk**: Gradual migration with testing at each phase
310+
4. **High reward**: More maintainable, extensible codebase
311+
312+
## Recommendation
313+
314+
**Proceed with implementation** following the phased approach outlined in [`implementation-plan.md`](./implementation-plan.md).
315+
316+
## Questions?
317+
318+
- Grammar details → [`grammar-refactor-proposal.md`](./grammar-refactor-proposal.md)
319+
- Visitor architecture → [`visitor-pattern-proposal.md`](./visitor-pattern-proposal.md)
320+
- Implementation steps → [`implementation-plan.md`](./implementation-plan.md)
321+
322+
## Appendix: Quick Reference
323+
324+
### Current State
325+
- ❌ Function calls don't parse
326+
- ❌ Grammar is ambiguous
327+
- ❌ Codegen is monolithic
328+
- ❌ Hard to add features
329+
330+
### After Refactoring
331+
- ✅ Function calls work
332+
- ✅ Grammar is clean
333+
- ✅ Codegen is modular
334+
- ✅ Easy to extend
335+
336+
### What Users Get
337+
```go
338+
// Calculator example with logging
339+
func handleOperator(state State, op string) {
340+
log(fmt.Sprintf("Operator: %s, State: %+v", op, state))
341+
342+
if state.Operator != "" {
343+
result := calculate(state)
344+
state.Display = formatNumber(result)
345+
log(fmt.Sprintf("Result: %f", result))
346+
}
347+
348+
state.Operator = op
349+
}
350+
```
351+
352+
**All of this just works!** 🎉

0 commit comments

Comments
 (0)