From fdef2814d0d7e65573d1b666ae9bc561fa35a9f9 Mon Sep 17 00:00:00 2001 From: _ov4 Date: Sun, 26 Jul 2026 12:11:00 +0000 Subject: refactor: extract ParseBlock/ParseExprList, resync-based error recovery Seven copy-pasted statement-block loops collapse into ParseBlock(terminators); five comma-separated expression-list loops collapse into ParseExprList(close, allowEmpty), which also fixes "arr[1 2] <- x" being silently accepted as arr[1,2] on the statement path. Error recovery now lives in one place: syncToStatementStart skips to the next statement-start/block-end token, so a bad statement no longer swallows the first token of the following good one. Drop the dead -1000/-1001 token check in ParsePrototypeArgs. Co-Authored-By: Claude Fable 5 --- AGENTS/CONVENTIONS.md | 6 +- AGENTS/EXTENDING.md | 8 ++- AGENTS/PITFALLS.md | 8 +-- include/cps/Parser.h | 4 ++ lib/Parser/Parser.cc | 151 +++++++++++++++++++++---------------------- lib/Parser/ParserFunction.cc | 34 ++-------- 6 files changed, 96 insertions(+), 115 deletions(-) diff --git a/AGENTS/CONVENTIONS.md b/AGENTS/CONVENTIONS.md index e09b6ee..753be57 100644 --- a/AGENTS/CONVENTIONS.md +++ b/AGENTS/CONVENTIONS.md @@ -54,9 +54,9 @@ keep compiling.** No exceptions, no exit mid-compile. `Lexer`/`Parser`/`CodeGen` expose `hadError()`; `CodeGen::reportError(fmt, ...)` is the helper for codegen-side diagnostics (`ArrayHandler` reaches it through its `CodeGen &` parameter, `FunctionGen`/`ArithmeticHandler` hold a `bool &HadError` reference). The driver exits -non-zero when any flag is set. The parser still drops a failed statement, skips one -token, and continues — later statements can be swallowed during recovery, but the -failure is always diagnosed and reflected in the exit code. +non-zero when any flag is set. The parser drops a failed statement and resyncs at the +next token that can start a statement or end a block (`syncToStatementStart`), then +continues; the failure is always diagnosed and reflected in the exit code. **Calls dispatch on the declared signature.** `FunctionGen::emitPrototype` records every function's pseudocode signature (`FuncSig`: return type name + per-param type name and diff --git a/AGENTS/EXTENDING.md b/AGENTS/EXTENDING.md index 3aab424..d5f9469 100644 --- a/AGENTS/EXTENDING.md +++ b/AGENTS/EXTENDING.md @@ -5,9 +5,11 @@ below assumes those invariants. **New statement:** token in `Lexer.h` enum + keyword match in `Lexer.cc`'s if-chain → node class in `AST.h` (unique_ptr children, move ctor, raw-ptr getters) → `ParseXxx` in -`Parser.cc` + branch in `ParseStatement` (copy the existing body-loop pattern for block -statements — terminator keyword + tok_eof guard) → `dynamic_cast` branch in -`CodeGen::emitStmt`. Guard every fall-through branch in control-flow codegen with +`Parser.cc` + branch in `ParseStatementImpl` (block statements parse their body with +`ParseBlock({terminator tokens})`; comma-separated expression lists use +`ParseExprList(close, allowEmpty)`; add the new statement's leading keyword to +`syncToStatementStart`) → `dynamic_cast` branch in `CodeGen::emitStmt`. Guard every +fall-through branch in control-flow codegen with `if (!Builder->GetInsertBlock()->getTerminator())` — a RETURN in the body may have already terminated the block. diff --git a/AGENTS/PITFALLS.md b/AGENTS/PITFALLS.md index 393f507..cd8cc92 100644 --- a/AGENTS/PITFALLS.md +++ b/AGENTS/PITFALLS.md @@ -7,10 +7,10 @@ test programs. - **STRING `=` / `<>` compares pointer identity, not contents.** No strcmp anywhere; two equal heap strings compare unequal. -- **OUTPUT takes exactly one expression.** `OUTPUT a, ", ", b` silently drops everything - after the comma — and if the dropped tail ends in an identifier, error recovery also - swallows the first token of the *next* statement. Concatenate with `&`. Similarly - INPUT only accepts a plain identifier — no `INPUT arr[i]`. +- **OUTPUT takes exactly one expression.** `OUTPUT a, ", ", b` prints `a`, then the + stray `, ", ", b` produces parse errors (recovery resyncs at the next statement + start, so following statements survive; the compile still fails). Concatenate with + `&`. Similarly INPUT only accepts a plain identifier — no `INPUT arr[i]`. - **Arrays cannot cross function boundaries.** `ArrayTable` is never scoped; metadata SSA values belong to the declaring function, so referencing another function's array produces invalid IR, and arrays can't be parameters. diff --git a/include/cps/Parser.h b/include/cps/Parser.h index c83be8b..7d19c77 100644 --- a/include/cps/Parser.h +++ b/include/cps/Parser.h @@ -2,6 +2,7 @@ #include "cps/Lexer.h" #include "cps/AST.h" #include +#include #include #include #include @@ -32,6 +33,9 @@ class Parser { std::unique_ptr ParseStatement(); std::unique_ptr ParseStatementImpl(); + std::vector> ParseBlock(std::initializer_list Terminators); + std::optional>> ParseExprList(char CloseDelim, bool AllowEmpty); + void syncToStatementStart(); std::unique_ptr ParseIfStmt(); std::unique_ptr ParseWhileStmt(); std::unique_ptr ParseRepeatStmt(); diff --git a/lib/Parser/Parser.cc b/lib/Parser/Parser.cc index 320fdf4..77df9bb 100644 --- a/lib/Parser/Parser.cc +++ b/lib/Parser/Parser.cc @@ -57,43 +57,18 @@ std::unique_ptr Parser::ParseIdentifierExpr() { if (CurTok == '(') { getNextToken(); - std::vector> Args; - if (CurTok != ')') { - while (true) { - if (auto Arg = ParseExpression()) - Args.push_back(std::move(Arg)); - else - return nullptr; - - if (CurTok == ')') break; - - if (CurTok != ',') { - fprintf(stderr, "Error: Expected ')' or ',' in function call.\n"); - return nullptr; - } - getNextToken(); - } - } - getNextToken(); - return std::make_unique(IdName, std::move(Args)); + auto Args = ParseExprList(')', true); + if (!Args) return nullptr; + return std::make_unique(IdName, std::move(*Args)); } - + if (CurTok == '[') { getNextToken(); - std::vector> Indices; - while (true) { - auto Exp = ParseExpression(); - if (!Exp) return nullptr; - Indices.push_back(std::move(Exp)); - if (CurTok == ']') break; - if (CurTok == ',') { getNextToken(); continue; } - fprintf(stderr, "Error: Expected ',' or ']'\n"); - return nullptr; - } - getNextToken(); - return std::make_unique(IdName, std::move(Indices), Line); + auto Indices = ParseExprList(']', false); + if (!Indices) return nullptr; + return std::make_unique(IdName, std::move(*Indices), Line); } - + return std::make_unique(IdName); } @@ -224,15 +199,67 @@ std::unique_ptr Parser::ParseExpression() { } std::vector> Parser::Parse() { - std::vector> Statements; + return ParseBlock({}); +} + +// After a statement fails to parse, skip ahead to the next token that can +// start a statement or end a block, so one bad statement cannot swallow the +// start of the next good one. +void Parser::syncToStatementStart() { while (CurTok != tok_eof) { + switch (CurTok) { + case tok_declare: case tok_identifier: case tok_input: case tok_output: + case tok_if: case tok_while: case tok_repeat: case tok_for: + case tok_function: case tok_procedure: case tok_call: case tok_return: + case tok_else: case tok_endif: case tok_endwhile: case tok_until: + case tok_next: case tok_endfunction: case tok_endprocedure: + return; + default: + getNextToken(); + } + } +} + +std::vector> Parser::ParseBlock(std::initializer_list Terminators) { + auto AtTerminator = [&] { + for (int T : Terminators) { + if (CurTok == T) return true; + } + return false; + }; + + std::vector> Body; + while (!AtTerminator() && CurTok != tok_eof) { if (auto Stmt = ParseStatement()) { - Statements.push_back(std::move(Stmt)); + Body.push_back(std::move(Stmt)); } else { - if (CurTok != tok_eof) getNextToken(); + syncToStatementStart(); } } - return Statements; + return Body; +} + +std::optional>> Parser::ParseExprList(char CloseDelim, bool AllowEmpty) { + std::vector> Elems; + if (AllowEmpty && CurTok == CloseDelim) { + getNextToken(); + return Elems; + } + + while (true) { + auto E = ParseExpression(); + if (!E) return std::nullopt; + Elems.push_back(std::move(E)); + + if (CurTok == CloseDelim) break; + if (CurTok != ',') { + fprintf(stderr, "Error: Expected ',' or '%c' at line %d\n", CloseDelim, Lex.getLine()); + return std::nullopt; + } + getNextToken(); + } + getNextToken(); + return Elems; } std::unique_ptr Parser::ParseDeclare() { @@ -318,21 +345,12 @@ std::unique_ptr Parser::ParseIfStmt() { } getNextToken(); - std::vector> ThenStmts; - while (CurTok != tok_else && CurTok != tok_endif && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) ThenStmts.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_else && CurTok != tok_endif) getNextToken(); - } + std::vector> ThenStmts = ParseBlock({tok_else, tok_endif}); std::vector> ElseStmts; if (CurTok == tok_else) { getNextToken(); - while (CurTok != tok_endif && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) ElseStmts.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_endif) getNextToken(); - } + ElseStmts = ParseBlock({tok_endif}); } if (CurTok != tok_endif) { @@ -355,12 +373,7 @@ std::unique_ptr Parser::ParseWhileStmt() { } getNextToken(); - std::vector> Body; - while (CurTok != tok_endwhile && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) Body.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_endwhile) getNextToken(); - } + std::vector> Body = ParseBlock({tok_endwhile}); if (CurTok != tok_endwhile) { fprintf(stderr, "Error: expected ENDWHILE\n"); @@ -374,12 +387,7 @@ std::unique_ptr Parser::ParseWhileStmt() { std::unique_ptr Parser::ParseRepeatStmt() { getNextToken(); - std::vector> Body; - while (CurTok != tok_until && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) Body.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_until) getNextToken(); - } + std::vector> Body = ParseBlock({tok_until}); if (CurTok != tok_until) { fprintf(stderr, "Error: expected UNTIL\n"); @@ -427,12 +435,7 @@ std::unique_ptr Parser::ParseForStmt() { if (!Step) return nullptr; } - std::vector> Body; - while (CurTok != tok_next && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) Body.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_next) getNextToken(); - } + std::vector> Body = ParseBlock({tok_next}); if (CurTok != tok_next) { fprintf(stderr, "Error: expected NEXT\n"); @@ -472,16 +475,9 @@ std::unique_ptr Parser::ParseStatementImpl() { if (CurTok == '[') { getNextToken(); - std::vector> Indices; - while (true) { - auto Exp = ParseExpression(); - if (!Exp) return nullptr; - Indices.push_back(std::move(Exp)); - if (CurTok == ']') break; - if (CurTok == ',') getNextToken(); - } - getNextToken(); - + auto Indices = ParseExprList(']', false); + if (!Indices) return nullptr; + if (CurTok != tok_assign) { fprintf(stderr, "Error: Expected '<-' after array access in assignment\n"); return nullptr; @@ -489,7 +485,7 @@ std::unique_ptr Parser::ParseStatementImpl() { getNextToken(); auto Expr = ParseExpression(); if (!Expr) return nullptr; - return std::make_unique(Name, std::move(Indices), std::move(Expr), Line); + return std::make_unique(Name, std::move(*Indices), std::move(Expr), Line); } if (CurTok != tok_assign) { @@ -543,5 +539,6 @@ std::unique_ptr Parser::ParseStatementImpl() { } fprintf(stderr, "Error: Unexpected token at line %d when expecting a statement\n", Lex.getLine()); + getNextToken(); // guarantee progress so error recovery cannot loop forever return nullptr; } diff --git a/lib/Parser/ParserFunction.cc b/lib/Parser/ParserFunction.cc index 06348c4..6ce0b8e 100644 --- a/lib/Parser/ParserFunction.cc +++ b/lib/Parser/ParserFunction.cc @@ -11,8 +11,6 @@ std::optional>> Parser::P getNextToken(); while (CurTok != ')') { - if (CurTok == -1000 || CurTok == -1001) getNextToken(); - bool IsRef = false; if (CurTok == tok_byref) { IsRef = true; @@ -79,12 +77,7 @@ std::unique_ptr Parser::ParseFunction() { auto Proto = std::make_unique(Name, std::move(*Args), RetType); - std::vector> Body; - while (CurTok != tok_endfunction && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) Body.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_endfunction) getNextToken(); - } + std::vector> Body = ParseBlock({tok_endfunction}); if (CurTok != tok_endfunction) { fprintf(stderr, "Error: expected ENDFUNCTION\n"); @@ -109,12 +102,7 @@ std::unique_ptr Parser::ParseProcedure() { if (!Args) return nullptr; auto Proto = std::make_unique(Name, std::move(*Args), "VOID"); - std::vector> Body; - while (CurTok != tok_endprocedure && CurTok != tok_eof) { - auto Stmt = ParseStatement(); - if (Stmt) Body.push_back(std::move(Stmt)); - else if (CurTok != tok_eof && CurTok != tok_endprocedure) getNextToken(); - } + std::vector> Body = ParseBlock({tok_endprocedure}); if (CurTok != tok_endprocedure) { fprintf(stderr, "Error: expected ENDPROCEDURE\n"); @@ -138,21 +126,11 @@ std::unique_ptr Parser::ParseCallStmt() { std::vector> Args; if (CurTok == '(') { getNextToken(); - while (CurTok != ')') { - auto Arg = ParseExpression(); - if (Arg) Args.push_back(std::move(Arg)); - else return nullptr; - - if (CurTok == ')') break; - if (CurTok != ',') { - fprintf(stderr, "Error: Expected ',' or ')' in call\n"); - return nullptr; - } - getNextToken(); - } - getNextToken(); + auto Parsed = ParseExprList(')', true); + if (!Parsed) return nullptr; + Args = std::move(*Parsed); } - + return std::make_unique(Callee, std::move(Args)); } -- cgit v1.3.1-4-g156e