From b40b20de12167bc5acc02428027e0d56c07fc0ee Mon Sep 17 00:00:00 2001 From: writemorecode Date: Sun, 8 Feb 2026 17:57:36 +0100 Subject: [PATCH 1/5] Add visitor infrastructure for AST traversal --- docs/symbol_table_visitor_spec.md | 268 ++++++++++++++++++++++++ src/ast/AstVisitor.cpp | 39 ++++ src/ast/AstVisitor.hpp | 25 +++ src/ast/ClassNode.cpp | 4 + src/ast/ClassNode.hpp | 5 + src/ast/MainClassNode.cpp | 4 + src/ast/MainClassNode.hpp | 10 + src/ast/MethodNode.cpp | 4 + src/ast/MethodNode.hpp | 8 + src/ast/MethodParameterNode.cpp | 6 + src/ast/MethodParameterNode.hpp | 10 + src/ast/MethodWithoutParametersNode.cpp | 6 + src/ast/MethodWithoutParametersNode.hpp | 6 + src/ast/Node.cpp | 4 + src/ast/Node.h | 4 + src/ast/VariableNode.cpp | 4 + src/ast/VariableNode.hpp | 9 + 17 files changed, 416 insertions(+) create mode 100644 docs/symbol_table_visitor_spec.md create mode 100644 src/ast/AstVisitor.cpp create mode 100644 src/ast/AstVisitor.hpp diff --git a/docs/symbol_table_visitor_spec.md b/docs/symbol_table_visitor_spec.md new file mode 100644 index 0000000..2fb3f9c --- /dev/null +++ b/docs/symbol_table_visitor_spec.md @@ -0,0 +1,268 @@ +# Symbol Table Visitor Refactor Spec + +## 1. Purpose +This spec defines a refactor from AST-owned symbol table construction (`Node::buildTable`) to a dedicated semantic pass using the Visitor pattern. The primary goals are: + +1. Centralize symbol table generation logic. +2. Make symbol table errors structured and testable. +3. Remove direct `stderr`/`stdout` side effects from symbol table construction. +4. Preserve current symbol table shape and downstream behavior. + +## 2. Current Problems +The current implementation has these issues: + +1. Symbol table behavior is spread across AST node classes (`ClassNode`, `MainClassNode`, `MethodNode`, `MethodWithoutParametersNode`, `MethodParameterNode`, `VariableNode`, plus recursive default in `Node`). +2. Error reporting is side-effect-only (`std::cerr` and one `std::cout`) and not inspectable as data. +3. `bool` return from `buildTable` loses error details (kind, location, count). +4. Compiler exit handling for semantic failure is inconsistent in `main.cpp`. + +## 3. Scope +In scope: + +1. New AST visitor infrastructure for semantic passes. +2. New `SymbolTableVisitor` pass. +3. Structured diagnostics for symbol-table errors. +4. Integration into compiler pipeline and tests. + +Out of scope for this change: + +1. Full type-checker migration to visitor. +2. IR generation migration to visitor. +3. Semantic language rules beyond existing symbol-table behavior. + +## 4. High-Level Architecture +### 4.1 New modules +Add these files: + +1. `src/ast/AstVisitor.hpp` +2. `src/semantic/SymbolTableVisitor.hpp` +3. `src/semantic/SymbolTableVisitor.cpp` + +### 4.2 Updated modules +Modify these files: + +1. `src/ast/Node.h` +2. `src/ast/Node.cpp` +3. `src/ast/ClassNode.hpp` +4. `src/ast/MainClassNode.hpp` +5. `src/ast/MethodNode.hpp` +6. `src/ast/MethodWithoutParametersNode.hpp` +7. `src/ast/MethodParameterNode.hpp` +8. `src/ast/VariableNode.hpp` +9. `src/main.cpp` +10. `tests/symbol_table_test.cpp` +11. `CMakeLists.txt` (if new compilation unit must be listed explicitly) + +## 5. Visitor Contract +### 5.1 `AstVisitor` interface +`AstVisitor` defines overloads for relevant node types and a fallback: + +```cpp +class AstVisitor { + public: + virtual ~AstVisitor() = default; + + virtual void visit(const Node &node); + virtual void visit(const ClassNode &node); + virtual void visit(const MainClassNode &node); + virtual void visit(const MethodNode &node); + virtual void visit(const MethodWithoutParametersNode &node); + virtual void visit(const MethodParameterNode &node); + virtual void visit(const VariableNode &node); +}; +``` + +Design intent: + +1. `visit(const Node&)` is default recursive traversal across `children`. +2. Specialized overloads handle symbol-table declarations with scope-aware logic. + +### 5.2 `Node::accept` +Add to `Node`: + +```cpp +virtual void accept(AstVisitor &visitor) const; +``` + +Implementation: + +1. Base `Node::accept` calls `visitor.visit(*this)`. +2. Relevant subclasses override `accept` and call their typed overload. +3. Non-specialized nodes inherit base behavior. + +## 6. Node Data Access Requirements +`SymbolTableVisitor` must not parse semantics from string labels like `node.type == "Method"`. It must use typed nodes. + +Add const getters for symbol-table-relevant nodes: + +1. `ClassNode`: class name, body node. +2. `MainClassNode`: class name, main argument name, body node. +3. `MethodNode`: method name, method return type string, params node, body node. +4. `MethodWithoutParametersNode`: method name, return type string, body node. +5. `MethodParameterNode`: parameter type string, parameter name. +6. `VariableNode`: variable type string, variable name. + +Getters should return `const std::string&` and/or `const Node&` where applicable. + +## 7. Diagnostics Model +### 7.1 Reuse existing diagnostics infrastructure +Reuse `lexing::Diagnostic` and `lexing::DiagnosticSink` from `src/lexing/Diagnostics.hpp`. + +For semantic diagnostics: + +1. `severity` is always `lexing::Severity::Error` for this pass. +2. `span.begin.line` and `span.end.line` are set from node line number. +3. `message` matches current user-facing style where practical. + +### 7.2 Pass result type +Define in `SymbolTableVisitor.hpp`: + +```cpp +struct SemanticPassResult { + int error_count = 0; + [[nodiscard]] bool ok() const { return error_count == 0; } +}; +``` + +Public entry point: + +```cpp +SemanticPassResult build_symbol_table(const Node &root, SymbolTable &table, + lexing::DiagnosticSink *sink = nullptr); +``` + +Behavior: + +1. Pass emits diagnostics to `sink` if provided. +2. Pass tracks its own `error_count` regardless of sink presence. +3. No direct printing in pass code. + +## 8. Symbol Table Construction Rules +These rules preserve current semantics unless explicitly stated. + +### 8.1 Program traversal +1. Start from root AST node. +2. Traverse in AST child order. +3. Build scopes and records deterministically. + +### 8.2 Main class declaration +1. On duplicate class name in program scope, emit error and skip main-class subtree. +2. Otherwise: +3. Add class record. +4. Enter class scope. +5. Add `this` variable with class type and attach to class record. +6. Add `main` method with return type `void` and attach to class record. +7. Enter method scope. +8. Add main argument variable of type `String[]`. +9. Exit method scope. +10. Visit main body statements. +11. Exit class scope. + +### 8.3 Regular class declaration +1. On duplicate class name in program scope, emit error and skip class body traversal. +2. Otherwise: +3. Add class record. +4. Enter class scope. +5. Add `this` variable and attach to class record. +6. Visit class body. +7. Exit class scope. + +### 8.4 Method declaration with parameters +1. Duplicate check is class-scope-local. +2. On duplicate method name in current class, emit error and skip method subtree. +3. Otherwise add method record and attach to class record. +4. Enter method scope. +5. Visit parameter list and body. +6. Exit method scope. + +### 8.5 Method declaration without parameters +1. Same duplicate and scope behavior as 8.4. +2. Visit body only. + +### 8.6 Parameter declaration +1. Duplicate check is current method scope only. +2. On duplicate parameter name, emit error and skip insertion. +3. Otherwise add variable, then attach as method parameter. + +### 8.7 Variable declaration +1. Duplicate check is current scope only. +2. On duplicate variable name, emit error and skip insertion. +3. Otherwise add variable, then attach to current record: +4. If current record is class, add as class field. +5. If current record is method, add as method local. + +## 9. Error Recovery Policy +The pass must continue after recoverable errors to maximize diagnostics per run. + +Rules: + +1. Duplicate class: skip that class subtree, continue siblings. +2. Duplicate method: skip that method subtree, continue siblings. +3. Duplicate variable/parameter: skip insertion only, continue same subtree. +4. Scope push/pop must remain balanced even when errors occur. + +Implementation requirement: + +1. Use a local RAII scope guard for `enter*Scope`/`exitScope` pairs. + +## 10. Backward Compatibility and Migration +### 10.1 Phase 1 migration target +1. Introduce visitor pass and switch all callers from `root->buildTable(st)` to `build_symbol_table(*root, st, sink)`. +2. Keep `Node::buildTable` temporarily as compatibility wrapper if needed. +3. Mark `Node::buildTable` and overrides deprecated in comments. + +### 10.2 Phase 2 cleanup +1. Remove `buildTable` virtual function from AST base and subclasses. +2. Remove old implementations in node `.cpp` files. +3. Keep behavioral parity validated by tests. + +## 11. Compiler Pipeline Integration +`main.cpp` semantic stage must change to: + +1. Run symbol table pass with sink that prints diagnostics. +2. Run type checker pass only if symbol table pass is OK. +3. Return `errCodes::SEMANTIC_ERROR` on symbol-table or type-check errors. + +This corrects current semantic failure exit behavior. + +## 12. Test Plan +### 12.1 Existing tests to keep +Keep current golden symbol table tests and adapt them to the new API. + +### 12.2 New tests to add +Add tests in `tests/symbol_table_test.cpp` for diagnostics: + +1. Duplicate class emits one error with expected line. +2. Duplicate method in same class emits one error with expected line. +3. Duplicate parameter emits one error with expected line. +4. Duplicate local variable emits one error with expected line. +5. Duplicate field emits one error with expected line. + +### 12.3 Test style +Use collecting sink pattern already present in tests: + +1. Parse source. +2. Run `build_symbol_table` with collecting sink. +3. Assert `SemanticPassResult`. +4. Assert diagnostic count, line(s), and key message fragments. + +## 13. Acceptance Criteria +The refactor is complete when all are true: + +1. No symbol-table code writes directly to `std::cerr` or `std::cout`. +2. Symbol table generation is executed via `SymbolTableVisitor` pass API. +3. Existing symbol table golden tests pass with equivalent scope/record layout. +4. New negative tests validate diagnostics and recovery. +5. Compiler returns `SEMANTIC_ERROR` on symbol-table failure. + +## 14. Non-Goals and Follow-Up +Non-goal now: + +1. Complete migration of `checkTypes` to visitor. + +Recommended follow-up: + +1. Introduce `TypeCheckVisitor` with same diagnostics contract. +2. Unify semantic pass runner API for all semantic phases. +3. Replace `std::string` type-check result with structured result + diagnostics. + diff --git a/src/ast/AstVisitor.cpp b/src/ast/AstVisitor.cpp new file mode 100644 index 0000000..664796a --- /dev/null +++ b/src/ast/AstVisitor.cpp @@ -0,0 +1,39 @@ +#include "ast/AstVisitor.hpp" + +#include "ast/ClassNode.hpp" +#include "ast/MainClassNode.hpp" +#include "ast/MethodNode.hpp" +#include "ast/MethodParameterNode.hpp" +#include "ast/MethodWithoutParametersNode.hpp" +#include "ast/Node.h" +#include "ast/VariableNode.hpp" + +void AstVisitor::visit(const Node &node) { + for (const auto &child : node.children) { + child->accept(*this); + } +} + +void AstVisitor::visit(const ClassNode &node) { + visit(static_cast(node)); +} + +void AstVisitor::visit(const MainClassNode &node) { + visit(static_cast(node)); +} + +void AstVisitor::visit(const MethodNode &node) { + visit(static_cast(node)); +} + +void AstVisitor::visit(const MethodWithoutParametersNode &node) { + visit(static_cast(node)); +} + +void AstVisitor::visit(const MethodParameterNode &node) { + visit(static_cast(node)); +} + +void AstVisitor::visit(const VariableNode &node) { + visit(static_cast(node)); +} diff --git a/src/ast/AstVisitor.hpp b/src/ast/AstVisitor.hpp new file mode 100644 index 0000000..f9712bb --- /dev/null +++ b/src/ast/AstVisitor.hpp @@ -0,0 +1,25 @@ +#ifndef AST_VISITOR_HPP +#define AST_VISITOR_HPP + +class Node; +class ClassNode; +class MainClassNode; +class MethodNode; +class MethodWithoutParametersNode; +class MethodParameterNode; +class VariableNode; + +class AstVisitor { + public: + virtual ~AstVisitor() = default; + + virtual void visit(const Node &node); + virtual void visit(const ClassNode &node); + virtual void visit(const MainClassNode &node); + virtual void visit(const MethodNode &node); + virtual void visit(const MethodWithoutParametersNode &node); + virtual void visit(const MethodParameterNode &node); + virtual void visit(const VariableNode &node); +}; + +#endif diff --git a/src/ast/ClassNode.cpp b/src/ast/ClassNode.cpp index 91901f0..ec9c157 100644 --- a/src/ast/ClassNode.cpp +++ b/src/ast/ClassNode.cpp @@ -1,5 +1,9 @@ #include "ast/ClassNode.hpp" +#include "ast/AstVisitor.hpp" + +void ClassNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } + bool ClassNode::buildTable(SymbolTable &st) const { bool valid = true; if (st.lookupClass(className)) { diff --git a/src/ast/ClassNode.hpp b/src/ast/ClassNode.hpp index d16353c..48a8789 100644 --- a/src/ast/ClassNode.hpp +++ b/src/ast/ClassNode.hpp @@ -14,6 +14,11 @@ class ClassNode : public Node { body = append_child(std::move(body_)); className = id->value; } + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getClassName() const { return className; } + [[nodiscard]] const Node &getBodyNode() const { return *body; } + bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; diff --git a/src/ast/MainClassNode.cpp b/src/ast/MainClassNode.cpp index ee14f93..5f154f1 100644 --- a/src/ast/MainClassNode.cpp +++ b/src/ast/MainClassNode.cpp @@ -1,5 +1,9 @@ #include "ast/MainClassNode.hpp" +#include "ast/AstVisitor.hpp" + +void MainClassNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } + bool MainClassNode::buildTable(SymbolTable &st) const { if (st.lookupClass(mainClassName)) { std::cerr << "Error: (line " << lineno << ") Class '" << mainClassName diff --git a/src/ast/MainClassNode.hpp b/src/ast/MainClassNode.hpp index 5992b4b..0c460c9 100644 --- a/src/ast/MainClassNode.hpp +++ b/src/ast/MainClassNode.hpp @@ -18,6 +18,16 @@ class MainClassNode : public Node { mainMethodArgumentName = arg->value; } + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getMainClassName() const { + return mainClassName; + } + [[nodiscard]] const std::string &getMainMethodArgumentName() const { + return mainMethodArgumentName; + } + [[nodiscard]] const Node &getBodyNode() const { return *body; } + bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; diff --git a/src/ast/MethodNode.cpp b/src/ast/MethodNode.cpp index 4d50fc8..24d50ca 100644 --- a/src/ast/MethodNode.cpp +++ b/src/ast/MethodNode.cpp @@ -1,5 +1,9 @@ #include "ast/MethodNode.hpp" +#include "ast/AstVisitor.hpp" + +void MethodNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } + bool MethodNode::buildTable(SymbolTable &st) const { bool valid = true; if (st.lookupMethod(methodName)) { diff --git a/src/ast/MethodNode.hpp b/src/ast/MethodNode.hpp index 1f09b82..51534c6 100644 --- a/src/ast/MethodNode.hpp +++ b/src/ast/MethodNode.hpp @@ -19,6 +19,14 @@ class MethodNode : public Node { methodName = id->value; methodType = type->value; } + + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getMethodName() const { return methodName; } + [[nodiscard]] const std::string &getMethodType() const { return methodType; } + [[nodiscard]] const Node &getParametersNode() const { return *params; } + [[nodiscard]] const Node &getBodyNode() const { return *body; } + bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; diff --git a/src/ast/MethodParameterNode.cpp b/src/ast/MethodParameterNode.cpp index 03a9be7..7dc8259 100644 --- a/src/ast/MethodParameterNode.cpp +++ b/src/ast/MethodParameterNode.cpp @@ -1,5 +1,11 @@ #include "ast/MethodParameterNode.hpp" +#include "ast/AstVisitor.hpp" + +void MethodParameterNode::accept(AstVisitor &visitor) const { + visitor.visit(*this); +} + bool MethodParameterNode::buildTable(SymbolTable &st) const { if (st.lookupVariableInScope(id->value)) { std::cerr << "Error: (line " << lineno << ") Parameter '" << id->value diff --git a/src/ast/MethodParameterNode.hpp b/src/ast/MethodParameterNode.hpp index 139872f..f2372fe 100644 --- a/src/ast/MethodParameterNode.hpp +++ b/src/ast/MethodParameterNode.hpp @@ -13,6 +13,16 @@ class MethodParameterNode : public Node { type = append_child(std::move(type_)); id = append_child(std::move(id_)); } + + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getParameterType() const { + return type->value; + } + [[nodiscard]] const std::string &getParameterName() const { + return id->value; + } + bool buildTable(SymbolTable &st) const override; }; diff --git a/src/ast/MethodWithoutParametersNode.cpp b/src/ast/MethodWithoutParametersNode.cpp index 785508d..8dbb565 100644 --- a/src/ast/MethodWithoutParametersNode.cpp +++ b/src/ast/MethodWithoutParametersNode.cpp @@ -1,5 +1,11 @@ #include "ast/MethodWithoutParametersNode.hpp" +#include "ast/AstVisitor.hpp" + +void MethodWithoutParametersNode::accept(AstVisitor &visitor) const { + visitor.visit(*this); +} + bool MethodWithoutParametersNode::buildTable(SymbolTable &st) const { if (st.lookupMethod(id->value)) { std::cerr << "Error: (line " << lineno << ") Method '" << id->value diff --git a/src/ast/MethodWithoutParametersNode.hpp b/src/ast/MethodWithoutParametersNode.hpp index 3fac9e4..0af98fd 100644 --- a/src/ast/MethodWithoutParametersNode.hpp +++ b/src/ast/MethodWithoutParametersNode.hpp @@ -16,6 +16,12 @@ class MethodWithoutParametersNode : public Node { body = append_child(std::move(body_)); } + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getMethodName() const { return id->value; } + [[nodiscard]] const std::string &getMethodType() const { return type->value; } + [[nodiscard]] const Node &getBodyNode() const { return *body; } + bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; diff --git a/src/ast/Node.cpp b/src/ast/Node.cpp index 59f4b9c..218409d 100644 --- a/src/ast/Node.cpp +++ b/src/ast/Node.cpp @@ -1,4 +1,6 @@ #include "ast/Node.h" + +#include "ast/AstVisitor.hpp" #include "semantic/SymbolTable.hpp" bool Node::buildTable(SymbolTable &st) const { @@ -32,6 +34,8 @@ Operand Node::generateIR(CFG &graph, SymbolTable &st) { return "foobar"; } +void Node::accept(AstVisitor &visitor) const { visitor.visit(*this); } + void Node::print(int depth = 0) const { for (int i = 0; i < depth; i++) { std::cout << " "; diff --git a/src/ast/Node.h b/src/ast/Node.h index 2f4a0f7..0e2b0e9 100644 --- a/src/ast/Node.h +++ b/src/ast/Node.h @@ -12,6 +12,8 @@ #include "ir/CFG.hpp" +class AstVisitor; + class Node { public: std::string type{}, value{}; @@ -31,6 +33,8 @@ class Node { virtual Operand generateIR(CFG &graph, SymbolTable &st); + virtual void accept(AstVisitor &visitor) const; + void print(int depth) const; void printGraphviz(int &count, std::ostream &outStream); diff --git a/src/ast/VariableNode.cpp b/src/ast/VariableNode.cpp index 427c2b4..8efaa33 100644 --- a/src/ast/VariableNode.cpp +++ b/src/ast/VariableNode.cpp @@ -1,5 +1,9 @@ #include "ast/VariableNode.hpp" +#include "ast/AstVisitor.hpp" + +void VariableNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } + bool VariableNode::buildTable(SymbolTable &st) const { Variable *lookup = st.lookupVariableInScope(name->value); diff --git a/src/ast/VariableNode.hpp b/src/ast/VariableNode.hpp index 88bde80..46ffdf6 100644 --- a/src/ast/VariableNode.hpp +++ b/src/ast/VariableNode.hpp @@ -14,6 +14,15 @@ class VariableNode : public Node { name = append_child(std::move(name_)); } + void accept(AstVisitor &visitor) const override; + + [[nodiscard]] const std::string &getVariableType() const { + return type->value; + } + [[nodiscard]] const std::string &getVariableName() const { + return name->value; + } + bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; }; From a2420c328112aabaf4f52b84b83a0175500386c0 Mon Sep 17 00:00:00 2001 From: writemorecode Date: Sun, 8 Feb 2026 17:59:21 +0100 Subject: [PATCH 2/5] Add symbol table visitor semantic pass --- src/main.cpp | 18 ++- src/semantic/SymbolTableVisitor.cpp | 226 ++++++++++++++++++++++++++++ src/semantic/SymbolTableVisitor.hpp | 51 +++++++ 3 files changed, 289 insertions(+), 6 deletions(-) create mode 100644 src/semantic/SymbolTableVisitor.cpp create mode 100644 src/semantic/SymbolTableVisitor.hpp diff --git a/src/main.cpp b/src/main.cpp index fc09234..debeace 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,6 +15,7 @@ namespace fs = std::filesystem; #include "lexing/SourceBuffer.hpp" #include "lexing/StringViewStream.hpp" #include "parsing/Parser.hpp" +#include "semantic/SymbolTableVisitor.hpp" std::unique_ptr root; int lexical_errors = 0; @@ -273,19 +274,24 @@ int main(int argc, char **argv) { } SymbolTable st; - bool symbolTableSuccess = root->buildTable(st); + lexing::LegacyDiagnosticSink semantic_diag; + auto symbol_table_result = build_symbol_table(*root, st, &semantic_diag); + bool symbolTableSuccess = symbol_table_result.ok(); if (!symbolTableSuccess) { std::cout << "Symbol table construction failed.\n"; } - auto rootType = root->checkTypes(st); - bool typeCheckSuccess = !rootType.empty(); - if (!typeCheckSuccess) { - std::cout << "Type checking failed.\n"; + bool typeCheckSuccess = true; + if (symbolTableSuccess) { + auto rootType = root->checkTypes(st); + typeCheckSuccess = !rootType.empty(); + if (!typeCheckSuccess) { + std::cout << "Type checking failed.\n"; + } } if (!symbolTableSuccess || !typeCheckSuccess) { - return errCode; + return errCodes::SEMANTIC_ERROR; } std::ofstream outStream(outputDirectory / "tree.dot"); diff --git a/src/semantic/SymbolTableVisitor.cpp b/src/semantic/SymbolTableVisitor.cpp new file mode 100644 index 0000000..8d522cb --- /dev/null +++ b/src/semantic/SymbolTableVisitor.cpp @@ -0,0 +1,226 @@ +#include "semantic/SymbolTableVisitor.hpp" + +#include +#include +#include +#include + +#include "ast/ClassNode.hpp" +#include "ast/MainClassNode.hpp" +#include "ast/MethodNode.hpp" +#include "ast/MethodParameterNode.hpp" +#include "ast/MethodWithoutParametersNode.hpp" +#include "ast/Node.h" +#include "ast/VariableNode.hpp" +#include "semantic/Class.hpp" +#include "semantic/Method.hpp" +#include "semantic/Record.hpp" +#include "semantic/Variable.hpp" + +namespace { + +class ScopeExit { + public: + explicit ScopeExit(SymbolTable &table) : table_(&table) {} + + ~ScopeExit() { + if (table_ != nullptr) { + table_->exitScope(); + } + } + + ScopeExit(const ScopeExit &) = delete; + ScopeExit &operator=(const ScopeExit &) = delete; + + private: + SymbolTable *table_ = nullptr; +}; + +} // namespace + +void SymbolTableVisitor::emit_error(int line, std::string message) { + error_count_ += 1; + + if (sink_ == nullptr) { + return; + } + + const auto line_no = static_cast(std::max(line, 1)); + const lexing::SourceSpan span{ + .begin = {.offset = 0, .line = line_no, .column = 1}, + .end = {.offset = 0, .line = line_no, .column = 1}, + }; + + sink_->emit({.severity = lexing::Severity::Error, + .message = std::move(message), + .span = span}); +} + +void SymbolTableVisitor::visit(const Node &node) { + for (const auto &child : node.children) { + child->accept(*this); + } +} + +void SymbolTableVisitor::visit(const ClassNode &node) { + const auto &class_name = node.getClassName(); + + if (table_.lookupClass(class_name) != nullptr) { + emit_error(node.lineno, "Error: (line " + std::to_string(node.lineno) + + ") Class '" + class_name + + "' already declared.\n"); + return; + } + + table_.addClass(class_name); + auto *current_class = table_.lookupClass(class_name); + table_.enterClassScope(current_class); + ScopeExit exit_scope(table_); + + table_.addVariable(class_name, "this"); + auto *this_variable = table_.lookupVariableInScope("this"); + current_class->addVariable(this_variable); + + node.getBodyNode().accept(*this); +} + +void SymbolTableVisitor::visit(const MainClassNode &node) { + const auto &main_class_name = node.getMainClassName(); + + if (table_.lookupClass(main_class_name) != nullptr) { + emit_error(node.lineno, + "Error: (line " + std::to_string(node.lineno) + + ") Class '" + main_class_name + + "' already declared.\n"); + return; + } + + table_.addClass(main_class_name); + auto *main_class = table_.lookupClass(main_class_name); + table_.enterClassScope(main_class); + ScopeExit exit_class_scope(table_); + + table_.addVariable(main_class_name, "this"); + auto *main_class_this = table_.lookupVariableInScope("this"); + main_class->addVariable(main_class_this); + + table_.addMethod("void", "main"); + auto *main_class_method = table_.lookupMethod("main"); + main_class->addMethod(main_class_method); + + table_.enterMethodScope(main_class_method); + { + ScopeExit exit_method_scope(table_); + table_.addVariable("String[]", node.getMainMethodArgumentName()); + } + + node.getBodyNode().accept(*this); +} + +void SymbolTableVisitor::visit(const MethodNode &node) { + auto *current_class = dynamic_cast(table_.getCurrentRecord()); + const auto &method_name = node.getMethodName(); + + if (current_class != nullptr && current_class->lookupMethod(method_name)) { + emit_error(node.lineno, + "Error: (line " + std::to_string(node.lineno) + + ") Method '" + method_name + "' already declared.\n"); + return; + } + + table_.addMethod(node.getMethodType(), method_name); + auto *current_method = table_.lookupMethod(method_name); + + if (current_class != nullptr) { + current_class->addMethod(current_method); + } + + table_.enterMethodScope(current_method); + ScopeExit exit_method_scope(table_); + + node.getParametersNode().accept(*this); + node.getBodyNode().accept(*this); +} + +void SymbolTableVisitor::visit(const MethodWithoutParametersNode &node) { + auto *current_class = dynamic_cast(table_.getCurrentRecord()); + const auto &method_name = node.getMethodName(); + + if (current_class != nullptr && current_class->lookupMethod(method_name)) { + emit_error(node.lineno, + "Error: (line " + std::to_string(node.lineno) + + ") Method '" + method_name + "' already declared.\n"); + return; + } + + table_.addMethod(node.getMethodType(), method_name); + auto *current_method = table_.lookupMethod(method_name); + + if (current_class != nullptr) { + current_class->addMethod(current_method); + } + + table_.enterMethodScope(current_method); + ScopeExit exit_method_scope(table_); + + node.getBodyNode().accept(*this); +} + +void SymbolTableVisitor::visit(const MethodParameterNode &node) { + const auto ¶meter_name = node.getParameterName(); + + if (table_.lookupVariableInScope(parameter_name) != nullptr) { + emit_error(node.lineno, + "Error: (line " + std::to_string(node.lineno) + + ") Parameter '" + parameter_name + + "' already declared.\n"); + return; + } + + table_.addVariable(node.getParameterType(), parameter_name); + auto *parameter = table_.lookupVariable(parameter_name); + + auto *current_scope = table_.getCurrentScope(); + auto *current_method = + dynamic_cast(current_scope != nullptr ? current_scope->getRecord() + : nullptr); + if (current_method != nullptr) { + current_method->addParameter(parameter); + } +} + +void SymbolTableVisitor::visit(const VariableNode &node) { + const auto &variable_name = node.getVariableName(); + + if (table_.lookupVariableInScope(variable_name) != nullptr) { + emit_error(node.lineno, + "Error: (line " + std::to_string(node.lineno) + + ") Variable '" + variable_name + + "' already declared.\n"); + return; + } + + table_.addVariable(node.getVariableType(), variable_name); + auto *current_variable = table_.lookupVariable(variable_name); + + auto *current_record = table_.getCurrentRecord(); + if (auto *current_class = dynamic_cast(current_record)) { + current_class->addVariable(current_variable); + return; + } + + if (auto *current_method = dynamic_cast(current_record)) { + current_method->addVariable(current_variable); + } +} + +SemanticPassResult build_symbol_table(const Node &root, SymbolTable &table, + lexing::DiagnosticSink *sink) { + while (table.getParentScope() != nullptr) { + table.exitScope(); + } + + SymbolTableVisitor visitor(table, sink); + root.accept(visitor); + return visitor.result(); +} diff --git a/src/semantic/SymbolTableVisitor.hpp b/src/semantic/SymbolTableVisitor.hpp new file mode 100644 index 0000000..6e805ec --- /dev/null +++ b/src/semantic/SymbolTableVisitor.hpp @@ -0,0 +1,51 @@ +#ifndef SYMBOL_TABLE_VISITOR_HPP +#define SYMBOL_TABLE_VISITOR_HPP + +#include + +#include "ast/AstVisitor.hpp" +#include "lexing/Diagnostics.hpp" +#include "semantic/SymbolTable.hpp" + +class Node; +class ClassNode; +class MainClassNode; +class MethodNode; +class MethodWithoutParametersNode; +class MethodParameterNode; +class VariableNode; + +struct SemanticPassResult { + int error_count = 0; + + [[nodiscard]] bool ok() const { return error_count == 0; } +}; + +class SymbolTableVisitor : public AstVisitor { + public: + explicit SymbolTableVisitor(SymbolTable &table, + lexing::DiagnosticSink *sink = nullptr) + : table_(table), sink_(sink) {} + + SemanticPassResult result() const { return {.error_count = error_count_}; } + + void visit(const Node &node) override; + void visit(const ClassNode &node) override; + void visit(const MainClassNode &node) override; + void visit(const MethodNode &node) override; + void visit(const MethodWithoutParametersNode &node) override; + void visit(const MethodParameterNode &node) override; + void visit(const VariableNode &node) override; + + private: + SymbolTable &table_; + lexing::DiagnosticSink *sink_ = nullptr; + int error_count_ = 0; + + void emit_error(int line, std::string message); +}; + +SemanticPassResult build_symbol_table(const Node &root, SymbolTable &table, + lexing::DiagnosticSink *sink = nullptr); + +#endif From ef1ca0e26440af51916c5f0c62b14b108af97b4a Mon Sep 17 00:00:00 2001 From: writemorecode Date: Sun, 8 Feb 2026 18:00:20 +0100 Subject: [PATCH 3/5] Update symbol table tests for visitor pass --- tests/symbol_table_test.cpp | 201 +++++++++++++++++++++++++++++++++++- 1 file changed, 199 insertions(+), 2 deletions(-) diff --git a/tests/symbol_table_test.cpp b/tests/symbol_table_test.cpp index d97ce01..8f6c859 100644 --- a/tests/symbol_table_test.cpp +++ b/tests/symbol_table_test.cpp @@ -16,6 +16,7 @@ #include "semantic/Method.hpp" #include "semantic/Scope.hpp" #include "semantic/SymbolTable.hpp" +#include "semantic/SymbolTableVisitor.hpp" namespace { @@ -37,6 +38,27 @@ void assert_no_errors(const std::vector &diagnostics) { << "Unexpected diagnostic errors: " << error_count; } +int count_error_diagnostics( + const std::vector &diagnostics) { + int count = 0; + for (const auto &d : diagnostics) { + if (d.severity == lexing::Severity::Error) { + count += 1; + } + } + return count; +} + +const lexing::Diagnostic * +find_first_error(const std::vector &diagnostics) { + for (const auto &d : diagnostics) { + if (d.severity == lexing::Severity::Error) { + return &d; + } + } + return nullptr; +} + std::unique_ptr parse_program(std::string_view source) { CollectingDiagnosticSink diag; auto stream = std::make_unique(source); @@ -164,7 +186,10 @@ class Foo { ASSERT_NE(root, nullptr); SymbolTable st; - ASSERT_TRUE(root->buildTable(st)); + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + ASSERT_TRUE(symbol_result.ok()); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 0); const Scope *program = st.getCurrentScope(); ASSERT_NE(program, nullptr); @@ -247,7 +272,10 @@ TEST(SymbolTable, GoldenProgram2) { ASSERT_NE(root, nullptr); SymbolTable st; - ASSERT_TRUE(root->buildTable(st)); + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + ASSERT_TRUE(symbol_result.ok()); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 0); const Scope *program = st.getCurrentScope(); ASSERT_NE(program, nullptr); @@ -439,3 +467,172 @@ TEST(SymbolTable, GoldenProgram2) { const auto root_type = root->checkTypes(st); EXPECT_EQ(root_type, "void"); } + +TEST(SymbolTable, DuplicateClassReportsDiagnostic) { + constexpr std::string_view source = R"(public class Main { + public static void main(String[] args) { + System.out.println(0); + } +} + +class Foo { +} + +class Foo { +} +)"; + + auto root = parse_program(source); + ASSERT_NE(root, nullptr); + + SymbolTable st; + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + + ASSERT_FALSE(symbol_result.ok()); + ASSERT_EQ(symbol_result.error_count, 1); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 1); + + const auto *error = find_first_error(semantic_diag.diagnostics); + ASSERT_NE(error, nullptr); + EXPECT_EQ(error->span.begin.line, 10u); + EXPECT_NE(error->message.find("Class 'Foo' already declared"), + std::string::npos); +} + +TEST(SymbolTable, DuplicateMethodReportsDiagnostic) { + constexpr std::string_view source = R"(public class Main { + public static void main(String[] args) { + System.out.println(0); + } +} + +class Foo { + public int bar() { + return 0; + } + + public int bar() { + return 1; + } +} +)"; + + auto root = parse_program(source); + ASSERT_NE(root, nullptr); + + SymbolTable st; + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + + ASSERT_FALSE(symbol_result.ok()); + ASSERT_EQ(symbol_result.error_count, 1); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 1); + + const auto *error = find_first_error(semantic_diag.diagnostics); + ASSERT_NE(error, nullptr); + EXPECT_EQ(error->span.begin.line, 12u); + EXPECT_NE(error->message.find("Method 'bar' already declared"), + std::string::npos); +} + +TEST(SymbolTable, DuplicateParameterReportsDiagnostic) { + constexpr std::string_view source = R"(public class Main { + public static void main(String[] args) { + System.out.println(0); + } +} + +class Foo { + public int bar(int x, int x) { + return x; + } +} +)"; + + auto root = parse_program(source); + ASSERT_NE(root, nullptr); + + SymbolTable st; + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + + ASSERT_FALSE(symbol_result.ok()); + ASSERT_EQ(symbol_result.error_count, 1); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 1); + + const auto *error = find_first_error(semantic_diag.diagnostics); + ASSERT_NE(error, nullptr); + EXPECT_EQ(error->span.begin.line, 8u); + EXPECT_NE(error->message.find("Parameter 'x' already declared"), + std::string::npos); +} + +TEST(SymbolTable, DuplicateLocalVariableReportsDiagnostic) { + constexpr std::string_view source = R"(public class Main { + public static void main(String[] args) { + System.out.println(0); + } +} + +class Foo { + public int bar() { + int x; + int x; + return x; + } +} +)"; + + auto root = parse_program(source); + ASSERT_NE(root, nullptr); + + SymbolTable st; + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + + ASSERT_FALSE(symbol_result.ok()); + ASSERT_EQ(symbol_result.error_count, 1); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 1); + + const auto *error = find_first_error(semantic_diag.diagnostics); + ASSERT_NE(error, nullptr); + EXPECT_EQ(error->span.begin.line, 10u); + EXPECT_NE(error->message.find("Variable 'x' already declared"), + std::string::npos); +} + +TEST(SymbolTable, DuplicateFieldReportsDiagnostic) { + constexpr std::string_view source = R"(public class Main { + public static void main(String[] args) { + System.out.println(0); + } +} + +class Foo { + int x; + int x; + + public int bar() { + return x; + } +} +)"; + + auto root = parse_program(source); + ASSERT_NE(root, nullptr); + + SymbolTable st; + CollectingDiagnosticSink semantic_diag; + const auto symbol_result = build_symbol_table(*root, st, &semantic_diag); + + ASSERT_FALSE(symbol_result.ok()); + ASSERT_EQ(symbol_result.error_count, 1); + ASSERT_EQ(count_error_diagnostics(semantic_diag.diagnostics), 1); + + const auto *error = find_first_error(semantic_diag.diagnostics); + ASSERT_NE(error, nullptr); + EXPECT_EQ(error->span.begin.line, 9u); + EXPECT_NE(error->message.find("Variable 'x' already declared"), + std::string::npos); +} From 295b1050d4359dff3b142a7ad50ad85e11f495fa Mon Sep 17 00:00:00 2001 From: writemorecode Date: Sun, 8 Feb 2026 18:02:27 +0100 Subject: [PATCH 4/5] Remove node-specific symbol table builders --- src/ast/ClassNode.cpp | 19 ------------------ src/ast/ClassNode.hpp | 1 - src/ast/MainClassNode.cpp | 25 ------------------------ src/ast/MainClassNode.hpp | 1 - src/ast/MethodNode.cpp | 21 -------------------- src/ast/MethodNode.hpp | 1 - src/ast/MethodParameterNode.cpp | 14 ------------- src/ast/MethodParameterNode.hpp | 1 - src/ast/MethodWithoutParametersNode.cpp | 18 ----------------- src/ast/MethodWithoutParametersNode.hpp | 1 - src/ast/Node.cpp | 9 ++------- src/ast/VariableNode.cpp | 26 ------------------------- src/ast/VariableNode.hpp | 1 - 13 files changed, 2 insertions(+), 136 deletions(-) diff --git a/src/ast/ClassNode.cpp b/src/ast/ClassNode.cpp index ec9c157..afd7b49 100644 --- a/src/ast/ClassNode.cpp +++ b/src/ast/ClassNode.cpp @@ -4,25 +4,6 @@ void ClassNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } -bool ClassNode::buildTable(SymbolTable &st) const { - bool valid = true; - if (st.lookupClass(className)) { - std::cerr << "Error: "; - std::cerr << "(line " << lineno << ") "; - std::cerr << "Class " << className << " already declared.\n"; - valid = false; - } - - st.addClass(className); - auto *currentClass = st.lookupClass(className); - st.enterClassScope(currentClass); - st.addVariable(className, "this"); - bool validBody = body->buildTable(st); - st.exitScope(); - - return valid && validBody; -} - std::string ClassNode::checkTypes(SymbolTable &st) const { st.enterClassScope(className); auto type = body->checkTypes(st); diff --git a/src/ast/ClassNode.hpp b/src/ast/ClassNode.hpp index 48a8789..259da4d 100644 --- a/src/ast/ClassNode.hpp +++ b/src/ast/ClassNode.hpp @@ -19,7 +19,6 @@ class ClassNode : public Node { [[nodiscard]] const std::string &getClassName() const { return className; } [[nodiscard]] const Node &getBodyNode() const { return *body; } - bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; }; diff --git a/src/ast/MainClassNode.cpp b/src/ast/MainClassNode.cpp index 5f154f1..ac10baa 100644 --- a/src/ast/MainClassNode.cpp +++ b/src/ast/MainClassNode.cpp @@ -4,31 +4,6 @@ void MainClassNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } -bool MainClassNode::buildTable(SymbolTable &st) const { - if (st.lookupClass(mainClassName)) { - std::cerr << "Error: (line " << lineno << ") Class '" << mainClassName - << "' already declared.\n"; - return false; - } - st.addClass(mainClassName); - auto *mainClass = st.lookupClass(mainClassName); - st.enterClassScope(mainClass); - - st.addVariable(mainClassName, "this"); - Variable *mainClassThis = st.lookupVariableInScope("this"); - mainClass->addVariable(mainClassThis); - - st.addMethod("void", "main"); - Method *mainClassMethod = st.lookupMethod("main"); - mainClass->addMethod(mainClassMethod); - - st.enterMethodScope(mainClassMethod); - st.addVariable("String[]", mainMethodArgumentName); - st.exitScope(); - st.exitScope(); - return true; -} - std::string MainClassNode::checkTypes(SymbolTable &st) const { st.enterClassScope(mainClassName); body->checkTypes(st); diff --git a/src/ast/MainClassNode.hpp b/src/ast/MainClassNode.hpp index 0c460c9..24e8dcb 100644 --- a/src/ast/MainClassNode.hpp +++ b/src/ast/MainClassNode.hpp @@ -28,7 +28,6 @@ class MainClassNode : public Node { } [[nodiscard]] const Node &getBodyNode() const { return *body; } - bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; }; diff --git a/src/ast/MethodNode.cpp b/src/ast/MethodNode.cpp index 24d50ca..f2825a8 100644 --- a/src/ast/MethodNode.cpp +++ b/src/ast/MethodNode.cpp @@ -4,27 +4,6 @@ void MethodNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } -bool MethodNode::buildTable(SymbolTable &st) const { - bool valid = true; - if (st.lookupMethod(methodName)) { - std::cerr << "Error: (line " << lineno << ") Method '" << methodName - << "' already declared.\n"; - valid = false; - } - - st.addMethod(methodType, methodName); - auto *currentMethod = st.lookupMethod(methodName); - auto *currentClass = dynamic_cast(st.getCurrentRecord()); - currentClass->addMethod(currentMethod); - - st.enterMethodScope(currentMethod); - bool validParams = params->buildTable(st); - bool validBody = body->buildTable(st); - st.exitScope(); - - return valid && validParams && validBody; -} - std::string MethodNode::checkTypes(SymbolTable &st) const { st.enterMethodScope(methodName); const auto signatureReturnType = type->checkTypes(st); diff --git a/src/ast/MethodNode.hpp b/src/ast/MethodNode.hpp index 51534c6..5c5d008 100644 --- a/src/ast/MethodNode.hpp +++ b/src/ast/MethodNode.hpp @@ -27,7 +27,6 @@ class MethodNode : public Node { [[nodiscard]] const Node &getParametersNode() const { return *params; } [[nodiscard]] const Node &getBodyNode() const { return *body; } - bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; }; diff --git a/src/ast/MethodParameterNode.cpp b/src/ast/MethodParameterNode.cpp index 7dc8259..8196ac9 100644 --- a/src/ast/MethodParameterNode.cpp +++ b/src/ast/MethodParameterNode.cpp @@ -5,17 +5,3 @@ void MethodParameterNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } - -bool MethodParameterNode::buildTable(SymbolTable &st) const { - if (st.lookupVariableInScope(id->value)) { - std::cerr << "Error: (line " << lineno << ") Parameter '" << id->value - << "' already declared.\n"; - return false; - } - st.addVariable(type->value, id->value); - auto *parameter = st.lookupVariable(id->value); - auto *currentScope = st.getCurrentScope(); - auto *currentMethod = dynamic_cast(currentScope->getRecord()); - currentMethod->addParameter(parameter); - return true; -} diff --git a/src/ast/MethodParameterNode.hpp b/src/ast/MethodParameterNode.hpp index f2372fe..6f6ae01 100644 --- a/src/ast/MethodParameterNode.hpp +++ b/src/ast/MethodParameterNode.hpp @@ -23,7 +23,6 @@ class MethodParameterNode : public Node { return id->value; } - bool buildTable(SymbolTable &st) const override; }; #endif diff --git a/src/ast/MethodWithoutParametersNode.cpp b/src/ast/MethodWithoutParametersNode.cpp index 8dbb565..0f9cf71 100644 --- a/src/ast/MethodWithoutParametersNode.cpp +++ b/src/ast/MethodWithoutParametersNode.cpp @@ -6,24 +6,6 @@ void MethodWithoutParametersNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } -bool MethodWithoutParametersNode::buildTable(SymbolTable &st) const { - if (st.lookupMethod(id->value)) { - std::cerr << "Error: (line " << lineno << ") Method '" << id->value - << "' already declared.\n"; - return false; - } - st.addMethod(type->value, id->value); - auto *currentMethod = st.lookupMethod(id->value); - auto *currentClass = dynamic_cast(st.getCurrentRecord()); - currentClass->addMethod(currentMethod); - - st.enterMethodScope(currentMethod); - bool validBody = body->buildTable(st); - st.exitScope(); - - return validBody; -} - std::string MethodWithoutParametersNode::checkTypes(SymbolTable &st) const { st.enterMethodScope(id->value); const auto signatureReturnType = type->checkTypes(st); diff --git a/src/ast/MethodWithoutParametersNode.hpp b/src/ast/MethodWithoutParametersNode.hpp index 0af98fd..d90d663 100644 --- a/src/ast/MethodWithoutParametersNode.hpp +++ b/src/ast/MethodWithoutParametersNode.hpp @@ -22,7 +22,6 @@ class MethodWithoutParametersNode : public Node { [[nodiscard]] const std::string &getMethodType() const { return type->value; } [[nodiscard]] const Node &getBodyNode() const { return *body; } - bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; Operand generateIR(CFG &graph, SymbolTable &st) override; }; diff --git a/src/ast/Node.cpp b/src/ast/Node.cpp index 218409d..b58d78f 100644 --- a/src/ast/Node.cpp +++ b/src/ast/Node.cpp @@ -2,15 +2,10 @@ #include "ast/AstVisitor.hpp" #include "semantic/SymbolTable.hpp" +#include "semantic/SymbolTableVisitor.hpp" bool Node::buildTable(SymbolTable &st) const { - bool valid = true; - for (const auto &child : children) { - if (!child->buildTable(st)) { - valid = false; - } - } - return valid; + return build_symbol_table(*this, st).ok(); } std::string Node::checkTypes(SymbolTable &st) const { diff --git a/src/ast/VariableNode.cpp b/src/ast/VariableNode.cpp index 8efaa33..ea99317 100644 --- a/src/ast/VariableNode.cpp +++ b/src/ast/VariableNode.cpp @@ -4,32 +4,6 @@ void VariableNode::accept(AstVisitor &visitor) const { visitor.visit(*this); } -bool VariableNode::buildTable(SymbolTable &st) const { - - Variable *lookup = st.lookupVariableInScope(name->value); - if (lookup) { - std::cerr << "Error: (line " << lineno << ") " - << "Variable '" << name->value << "' " - << "already declared.\n"; - return false; - } - st.addVariable(type->value, name->value); - Variable *currentVariable = st.lookupVariable(name->value); - - Record *curRecord = st.getCurrentRecord(); - if (curRecord->getType() == curRecord->getID()) { - // Record is a class - auto *curClass = dynamic_cast(curRecord); - curClass->addVariable(currentVariable); - } else { - // Record is a method - auto *curMethod = dynamic_cast(curRecord); - curMethod->addVariable(currentVariable); - } - - return true; -}; - std::string VariableNode::checkTypes(SymbolTable &st) const { const auto &variableType = type->value; const bool hasBaseType = diff --git a/src/ast/VariableNode.hpp b/src/ast/VariableNode.hpp index 46ffdf6..4f1e9be 100644 --- a/src/ast/VariableNode.hpp +++ b/src/ast/VariableNode.hpp @@ -23,7 +23,6 @@ class VariableNode : public Node { return name->value; } - bool buildTable(SymbolTable &st) const override; std::string checkTypes(SymbolTable &st) const override; }; From fd798f6bed1ec1f4833bab8fad3765d06f14be1b Mon Sep 17 00:00:00 2001 From: writemorecode Date: Sun, 8 Feb 2026 20:13:24 +0100 Subject: [PATCH 5/5] Remove Codex refactor spec file --- docs/symbol_table_visitor_spec.md | 268 ------------------------------ 1 file changed, 268 deletions(-) delete mode 100644 docs/symbol_table_visitor_spec.md diff --git a/docs/symbol_table_visitor_spec.md b/docs/symbol_table_visitor_spec.md deleted file mode 100644 index 2fb3f9c..0000000 --- a/docs/symbol_table_visitor_spec.md +++ /dev/null @@ -1,268 +0,0 @@ -# Symbol Table Visitor Refactor Spec - -## 1. Purpose -This spec defines a refactor from AST-owned symbol table construction (`Node::buildTable`) to a dedicated semantic pass using the Visitor pattern. The primary goals are: - -1. Centralize symbol table generation logic. -2. Make symbol table errors structured and testable. -3. Remove direct `stderr`/`stdout` side effects from symbol table construction. -4. Preserve current symbol table shape and downstream behavior. - -## 2. Current Problems -The current implementation has these issues: - -1. Symbol table behavior is spread across AST node classes (`ClassNode`, `MainClassNode`, `MethodNode`, `MethodWithoutParametersNode`, `MethodParameterNode`, `VariableNode`, plus recursive default in `Node`). -2. Error reporting is side-effect-only (`std::cerr` and one `std::cout`) and not inspectable as data. -3. `bool` return from `buildTable` loses error details (kind, location, count). -4. Compiler exit handling for semantic failure is inconsistent in `main.cpp`. - -## 3. Scope -In scope: - -1. New AST visitor infrastructure for semantic passes. -2. New `SymbolTableVisitor` pass. -3. Structured diagnostics for symbol-table errors. -4. Integration into compiler pipeline and tests. - -Out of scope for this change: - -1. Full type-checker migration to visitor. -2. IR generation migration to visitor. -3. Semantic language rules beyond existing symbol-table behavior. - -## 4. High-Level Architecture -### 4.1 New modules -Add these files: - -1. `src/ast/AstVisitor.hpp` -2. `src/semantic/SymbolTableVisitor.hpp` -3. `src/semantic/SymbolTableVisitor.cpp` - -### 4.2 Updated modules -Modify these files: - -1. `src/ast/Node.h` -2. `src/ast/Node.cpp` -3. `src/ast/ClassNode.hpp` -4. `src/ast/MainClassNode.hpp` -5. `src/ast/MethodNode.hpp` -6. `src/ast/MethodWithoutParametersNode.hpp` -7. `src/ast/MethodParameterNode.hpp` -8. `src/ast/VariableNode.hpp` -9. `src/main.cpp` -10. `tests/symbol_table_test.cpp` -11. `CMakeLists.txt` (if new compilation unit must be listed explicitly) - -## 5. Visitor Contract -### 5.1 `AstVisitor` interface -`AstVisitor` defines overloads for relevant node types and a fallback: - -```cpp -class AstVisitor { - public: - virtual ~AstVisitor() = default; - - virtual void visit(const Node &node); - virtual void visit(const ClassNode &node); - virtual void visit(const MainClassNode &node); - virtual void visit(const MethodNode &node); - virtual void visit(const MethodWithoutParametersNode &node); - virtual void visit(const MethodParameterNode &node); - virtual void visit(const VariableNode &node); -}; -``` - -Design intent: - -1. `visit(const Node&)` is default recursive traversal across `children`. -2. Specialized overloads handle symbol-table declarations with scope-aware logic. - -### 5.2 `Node::accept` -Add to `Node`: - -```cpp -virtual void accept(AstVisitor &visitor) const; -``` - -Implementation: - -1. Base `Node::accept` calls `visitor.visit(*this)`. -2. Relevant subclasses override `accept` and call their typed overload. -3. Non-specialized nodes inherit base behavior. - -## 6. Node Data Access Requirements -`SymbolTableVisitor` must not parse semantics from string labels like `node.type == "Method"`. It must use typed nodes. - -Add const getters for symbol-table-relevant nodes: - -1. `ClassNode`: class name, body node. -2. `MainClassNode`: class name, main argument name, body node. -3. `MethodNode`: method name, method return type string, params node, body node. -4. `MethodWithoutParametersNode`: method name, return type string, body node. -5. `MethodParameterNode`: parameter type string, parameter name. -6. `VariableNode`: variable type string, variable name. - -Getters should return `const std::string&` and/or `const Node&` where applicable. - -## 7. Diagnostics Model -### 7.1 Reuse existing diagnostics infrastructure -Reuse `lexing::Diagnostic` and `lexing::DiagnosticSink` from `src/lexing/Diagnostics.hpp`. - -For semantic diagnostics: - -1. `severity` is always `lexing::Severity::Error` for this pass. -2. `span.begin.line` and `span.end.line` are set from node line number. -3. `message` matches current user-facing style where practical. - -### 7.2 Pass result type -Define in `SymbolTableVisitor.hpp`: - -```cpp -struct SemanticPassResult { - int error_count = 0; - [[nodiscard]] bool ok() const { return error_count == 0; } -}; -``` - -Public entry point: - -```cpp -SemanticPassResult build_symbol_table(const Node &root, SymbolTable &table, - lexing::DiagnosticSink *sink = nullptr); -``` - -Behavior: - -1. Pass emits diagnostics to `sink` if provided. -2. Pass tracks its own `error_count` regardless of sink presence. -3. No direct printing in pass code. - -## 8. Symbol Table Construction Rules -These rules preserve current semantics unless explicitly stated. - -### 8.1 Program traversal -1. Start from root AST node. -2. Traverse in AST child order. -3. Build scopes and records deterministically. - -### 8.2 Main class declaration -1. On duplicate class name in program scope, emit error and skip main-class subtree. -2. Otherwise: -3. Add class record. -4. Enter class scope. -5. Add `this` variable with class type and attach to class record. -6. Add `main` method with return type `void` and attach to class record. -7. Enter method scope. -8. Add main argument variable of type `String[]`. -9. Exit method scope. -10. Visit main body statements. -11. Exit class scope. - -### 8.3 Regular class declaration -1. On duplicate class name in program scope, emit error and skip class body traversal. -2. Otherwise: -3. Add class record. -4. Enter class scope. -5. Add `this` variable and attach to class record. -6. Visit class body. -7. Exit class scope. - -### 8.4 Method declaration with parameters -1. Duplicate check is class-scope-local. -2. On duplicate method name in current class, emit error and skip method subtree. -3. Otherwise add method record and attach to class record. -4. Enter method scope. -5. Visit parameter list and body. -6. Exit method scope. - -### 8.5 Method declaration without parameters -1. Same duplicate and scope behavior as 8.4. -2. Visit body only. - -### 8.6 Parameter declaration -1. Duplicate check is current method scope only. -2. On duplicate parameter name, emit error and skip insertion. -3. Otherwise add variable, then attach as method parameter. - -### 8.7 Variable declaration -1. Duplicate check is current scope only. -2. On duplicate variable name, emit error and skip insertion. -3. Otherwise add variable, then attach to current record: -4. If current record is class, add as class field. -5. If current record is method, add as method local. - -## 9. Error Recovery Policy -The pass must continue after recoverable errors to maximize diagnostics per run. - -Rules: - -1. Duplicate class: skip that class subtree, continue siblings. -2. Duplicate method: skip that method subtree, continue siblings. -3. Duplicate variable/parameter: skip insertion only, continue same subtree. -4. Scope push/pop must remain balanced even when errors occur. - -Implementation requirement: - -1. Use a local RAII scope guard for `enter*Scope`/`exitScope` pairs. - -## 10. Backward Compatibility and Migration -### 10.1 Phase 1 migration target -1. Introduce visitor pass and switch all callers from `root->buildTable(st)` to `build_symbol_table(*root, st, sink)`. -2. Keep `Node::buildTable` temporarily as compatibility wrapper if needed. -3. Mark `Node::buildTable` and overrides deprecated in comments. - -### 10.2 Phase 2 cleanup -1. Remove `buildTable` virtual function from AST base and subclasses. -2. Remove old implementations in node `.cpp` files. -3. Keep behavioral parity validated by tests. - -## 11. Compiler Pipeline Integration -`main.cpp` semantic stage must change to: - -1. Run symbol table pass with sink that prints diagnostics. -2. Run type checker pass only if symbol table pass is OK. -3. Return `errCodes::SEMANTIC_ERROR` on symbol-table or type-check errors. - -This corrects current semantic failure exit behavior. - -## 12. Test Plan -### 12.1 Existing tests to keep -Keep current golden symbol table tests and adapt them to the new API. - -### 12.2 New tests to add -Add tests in `tests/symbol_table_test.cpp` for diagnostics: - -1. Duplicate class emits one error with expected line. -2. Duplicate method in same class emits one error with expected line. -3. Duplicate parameter emits one error with expected line. -4. Duplicate local variable emits one error with expected line. -5. Duplicate field emits one error with expected line. - -### 12.3 Test style -Use collecting sink pattern already present in tests: - -1. Parse source. -2. Run `build_symbol_table` with collecting sink. -3. Assert `SemanticPassResult`. -4. Assert diagnostic count, line(s), and key message fragments. - -## 13. Acceptance Criteria -The refactor is complete when all are true: - -1. No symbol-table code writes directly to `std::cerr` or `std::cout`. -2. Symbol table generation is executed via `SymbolTableVisitor` pass API. -3. Existing symbol table golden tests pass with equivalent scope/record layout. -4. New negative tests validate diagnostics and recovery. -5. Compiler returns `SEMANTIC_ERROR` on symbol-table failure. - -## 14. Non-Goals and Follow-Up -Non-goal now: - -1. Complete migration of `checkTypes` to visitor. - -Recommended follow-up: - -1. Introduce `TypeCheckVisitor` with same diagnostics contract. -2. Unify semantic pass runner API for all semantic phases. -3. Replace `std::string` type-check result with structured result + diagnostics. -