From efc86b5c7625cf4c7074319f42f68807a25bde9d Mon Sep 17 00:00:00 2001 From: _ov4 Date: Sun, 26 Jul 2026 12:04:56 +0000 Subject: refactor: single builtin registry, retire keyword route for builtins All 11 builtins now live in one static {Name, Arity, ReturnTypeName} table in CodeGen.cc: emitExpr checks arity against it (uniform "Error: X expects N args" diagnostics) and getExprTypeInfo reads return types from it, removing the duplicated hard-coded table. LENGTH/MID/RIGHT/LEFT/LCASE/UCASE lose their lexer tokens and ParseStringBuiltin; they parse as ordinary identifier calls exactly like ASC/CHR/IS_NUM/NUM_TO_STR/STR_TO_NUM already did. Behavior note: these six names are no longer reserved words. AGENTS docs updated for the new builtin story and the batch-1 error contract (exit codes, verifyModule, fixed descending FOR). Co-Authored-By: Claude Fable 5 --- AGENTS/ARCH.md | 19 +++++++--------- AGENTS/CONVENTIONS.md | 29 ++++++++++++------------ AGENTS/EXTENDING.md | 10 +++++---- AGENTS/PITFALLS.md | 12 +++++----- include/cps/Lexer.h | 6 ----- include/cps/Parser.h | 2 -- lib/CodeGen/CodeGen.cc | 60 ++++++++++++++++++++++++++++++++++++-------------- lib/Lexer/Lexer.cc | 7 ------ lib/Parser/Parser.cc | 35 ----------------------------- 9 files changed, 77 insertions(+), 103 deletions(-) diff --git a/AGENTS/ARCH.md b/AGENTS/ARCH.md index 63087ac..a23297b 100644 --- a/AGENTS/ARCH.md +++ b/AGENTS/ARCH.md @@ -23,19 +23,16 @@ mkdir -p build && cd build && cmake -DLLVM_DIR=/usr/lib/llvm-18/cmake .. && make clang-18 prog.ll -o prog && ./prog ``` -Three facts about the driver (`tools/driver/main.cc`, 17 lines) that break naive automation: +Facts about the driver (`tools/driver/main.cc`) that break naive automation: - **No argv.** `./cpsc file.cps` blocks forever on stdin; you must redirect `<`. -- **IR and error messages share stderr.** A failed parse still dumps (partial) IR, - prefixed by `Error:` lines that make clang reject the file. -- **Exit code is always 0**, even on errors. Grepping stderr for `Error:` catches most - failures but has false negatives: some parse paths return nullptr without printing - anything (an identifier statement not followed by `<-`/`[`, an unrecognized statement - token), the keyword builtins' arity checks print without the `Error:` prefix - (`LENGTH expects 1 arg`), and the identifier-route builtins - (ASC/CHR/IS_NUM/NUM_TO_STR/STR_TO_NUM) fail arity checks silently. Those statements - just vanish from otherwise-valid IR. The only reliable check is running the program - and diffing its output. +- **IR and error messages share stderr.** A failed compile still dumps (partial) IR, + mixed with `Error:` lines that make clang reject the file. +- **Exit code is non-zero iff a diagnostic was emitted.** Lexer, Parser and CodeGen each + track an error flag (`hadError()`); every failure path prints an `Error:` line, and + `main` returns 1 when any flag is set. The emitted module is also checked with + `verifyModule`, so invalid IR flips the exit code too. Check the exit code — do not + grep stderr. Build pin: **LLVM 18 only.** LLVM 19 breaks compilation (`ArithmeticHandler.cc` uses `llvm::Module` methods relying on LLVM 18's transitive `#include`; fix would be adding diff --git a/AGENTS/CONVENTIONS.md b/AGENTS/CONVENTIONS.md index e3c67de..c05734f 100644 --- a/AGENTS/CONVENTIONS.md +++ b/AGENTS/CONVENTIONS.md @@ -49,12 +49,14 @@ maps around a function body, but `ArrayHandler::ArrayTable` is *not* scoped (see `emitExpr` walk a linear if-chain of casts. **A statement type missing from the chain is silently ignored** — no error. Adding an AST node always means touching these chains. -**Error style everywhere: `fprintf(stderr, ...)` then return nullptr/void, and keep -compiling.** No exceptions, no exit, no error counter. Most messages start with -`Error:`; the keyword builtins' arity checks in `emitExpr` lack the prefix, and the -identifier-route builtins' arity checks and several parse failures print nothing at all. -The parser drops a failed statement, skips one token, and continues, so erroneous -statements simply vanish from the program. +**Error style everywhere: print an `Error:` line, set the component's error flag, and +keep compiling.** No exceptions, no exit mid-compile. `Lexer`/`Parser`/`CodeGen` each +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. **BYREF is a pointer-typed parameter, inferred structurally at call sites.** `FunctionGen` emits pointer params for BYREF; call sites (in CodeGen, duplicated for @@ -67,11 +69,10 @@ garbage at runtime. Only declared-BYREF STRING parameters work, and STRING argum never be literals/expressions. Function names get no mangling — a pseudocode `FUNCTION printf` collides with libc. -**Builtins have two routes and one trap.** LENGTH/MID/RIGHT/LEFT/LCASE/UCASE are -reserved keywords with tokens, normalized back into ordinary `CallExprAST` by the -parser; ASC/CHR/IS_NUM/NUM_TO_STR/STR_TO_NUM need no lexer/parser support at all — -identifiers may contain `_` and `Name(args)` already parses as a call. Both routes are -intercepted by name-string comparison in `emitExpr`'s `CallExprAST` branch *before* the -user-function fallback. The trap: builtin return types are duplicated in -`getExprTypeInfo`'s hard-coded table — forget that second edit and OUTPUT prints the -result with the wrong format, with no compile error. +**Builtins are plain identifiers backed by one table.** All 11 builtins +(LENGTH/MID/RIGHT/LEFT/LCASE/UCASE/ASC/CHR/IS_NUM/NUM_TO_STR/STR_TO_NUM) parse as +ordinary calls — no lexer/parser support, and the names are not reserved words. The +static `Builtins` table at the top of `CodeGen.cc` ({Name, Arity, ReturnTypeName}) is +the single source of truth: `emitExpr`'s `CallExprAST` branch checks arity against it +before dispatching to the per-builtin emit branches (still an if-chain, ahead of the +user-function fallback), and `getExprTypeInfo` reads return types from it. diff --git a/AGENTS/EXTENDING.md b/AGENTS/EXTENDING.md index 62cdf40..3aab424 100644 --- a/AGENTS/EXTENDING.md +++ b/AGENTS/EXTENDING.md @@ -11,10 +11,12 @@ statements — terminator keyword + tok_eof guard) → `dynamic_cast` branch in `if (!Builder->GetInsertBlock()->getTerminator())` — a RETURN in the body may have already terminated the block. -**New builtin function:** implement IR emission as a method on the fitting handler -(String/Char/StringConversion), add the name intercept in `emitExpr`'s CallExprAST -section with an arg-count check, **and** add the name to `getExprTypeInfo`'s table. -Identifier-shaped names need nothing in lexer/parser. +**New builtin function:** add a `{Name, Arity, ReturnTypeName}` row to the static +`Builtins` table at the top of `CodeGen.cc` (single source of truth — arity checking in +`emitExpr` and the return type in `getExprTypeInfo` both read it), then implement IR +emission as a method on the fitting handler (String/Char/StringConversion) and add the +name intercept in `emitExpr`'s CallExprAST section. Builtins are plain identifiers — +nothing is needed in lexer/parser. **New binary operator:** token (if multi-char) → one line in `BinopPrecedence` (Parser ctor; levels: OR=3, AND=5, comparisons=10, `+ - &`=20, `* / DIV MOD`=40; diff --git a/AGENTS/PITFALLS.md b/AGENTS/PITFALLS.md index 3b123fe..b068dee 100644 --- a/AGENTS/PITFALLS.md +++ b/AGENTS/PITFALLS.md @@ -7,11 +7,6 @@ test programs. - **STRING `=` / `<>` compares pointer identity, not contents.** No strcmp anywhere; two equal heap strings compare unequal. -- **Descending FOR loops never work.** CodeGen picks the loop direction at compile time - by dynamic_casting the STEP to a negative `IntegerExprAST` — but `-1` parses as the - expression `0 - 1` (the lexer has no signed literals), so that branch is dead code and - *every* FOR compiles with an ascending `<=` test. `FOR i <- 5 TO 1 STEP -1` runs zero - iterations (empirically verified). - **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 @@ -29,8 +24,11 @@ test programs. inference prints garbage instead of erroring. - The lexer emits no newline tokens; the grammar is purely keyword-delimited. Bare `RETURN` is only recognized right before ENDIF/ELSE/ENDFUNCTION/ENDPROCEDURE. -- `verifyFunction` results are ignored; invalid modules still print. -- The FOR loop variable must be pre-DECLAREd as INTEGER; FOR reuses that alloca. +- Invalid modules still print their IR, but `verifyModule` runs at the end of + `compile()` and makes the driver exit non-zero. +- The FOR loop variable must be pre-DECLAREd as INTEGER; FOR reuses that alloca. The + loop direction is decided at run time from the STEP value's sign; STEP is evaluated + once per iteration in the condition block. ## Not implemented (9618 features) diff --git a/include/cps/Lexer.h b/include/cps/Lexer.h index f1def3e..8984398 100644 --- a/include/cps/Lexer.h +++ b/include/cps/Lexer.h @@ -42,13 +42,7 @@ enum Token { tok_not = -27, tok_string_kw = -30, - tok_length = -31, - tok_mid = -32, - tok_right = -33, - tok_lcase = -34, - tok_ucase = -35, tok_string_literal = -36, - tok_left = -37, tok_char_kw = -38, tok_char_literal = -39, diff --git a/include/cps/Parser.h b/include/cps/Parser.h index 87d1dee..c83be8b 100644 --- a/include/cps/Parser.h +++ b/include/cps/Parser.h @@ -29,8 +29,6 @@ class Parser { std::unique_ptr ParseIdentifierExpr(); std::unique_ptr ParseParenExpr(); std::string ParseTypeName(bool AllowVoid = false); - - std::unique_ptr ParseStringBuiltin(const std::string &FuncName); std::unique_ptr ParseStatement(); std::unique_ptr ParseStatementImpl(); diff --git a/lib/CodeGen/CodeGen.cc b/lib/CodeGen/CodeGen.cc index 439e894..581e4a9 100644 --- a/lib/CodeGen/CodeGen.cc +++ b/lib/CodeGen/CodeGen.cc @@ -9,6 +9,40 @@ using namespace llvm; using namespace cps; +namespace { + +// Single source of truth for the built-in functions: name, arity and return +// type. emitExpr checks arity against this table before dispatching, and +// getExprTypeInfo reads the return type from it. +struct BuiltinInfo { + const char *Name; + unsigned Arity; + const char *ReturnTypeName; +}; + +constexpr BuiltinInfo Builtins[] = { + {"LENGTH", 1, "INTEGER"}, + {"MID", 3, "STRING"}, + {"RIGHT", 2, "STRING"}, + {"LEFT", 2, "STRING"}, + {"LCASE", 1, "STRING"}, + {"UCASE", 1, "STRING"}, + {"ASC", 1, "INTEGER"}, + {"CHR", 1, "STRING"}, + {"IS_NUM", 1, "BOOLEAN"}, + {"NUM_TO_STR", 1, "STRING"}, + {"STR_TO_NUM", 1, "REAL"}, +}; + +const BuiltinInfo *findBuiltin(const std::string &Name) { + for (const auto &B : Builtins) { + if (Name == B.Name) return &B; + } + return nullptr; +} + +} // namespace + CodeGen::~CodeGen() = default; void CodeGen::reportError(const char *Fmt, ...) { @@ -174,12 +208,9 @@ const TypeInfo *CodeGen::getExprTypeInfo(ExprAST *Expr) const { if (auto *Call = dynamic_cast(Expr)) { const std::string &Name = Call->getCallee(); - if (Name == "LENGTH" || Name == "ASC") return resolveType("INTEGER"); - if (Name == "MID" || Name == "RIGHT" || Name == "LEFT" || - Name == "LCASE" || Name == "UCASE" || Name == "CHR" || - Name == "NUM_TO_STR") return resolveType("STRING"); - if (Name == "IS_NUM") return resolveType("BOOLEAN"); - if (Name == "STR_TO_NUM") return resolveType("REAL"); + if (const BuiltinInfo *B = findBuiltin(Name)) { + return resolveType(B->ReturnTypeName); + } Function *CalleeF = TheModule->getFunction(Name); if (!CalleeF) return resolveType("INTEGER"); @@ -474,12 +505,16 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { if (auto *Call = dynamic_cast(Expr)) { std::string Name = Call->getCallee(); + if (const BuiltinInfo *B = findBuiltin(Name)) { + if (Call->getArgs().size() != B->Arity) { + reportError("%s expects %u arg%s", B->Name, B->Arity, B->Arity == 1 ? "" : "s"); + return nullptr; + } + } if (Name == "LENGTH") { - if (Call->getArgs().size() != 1) { reportError("LENGTH expects 1 arg"); return nullptr; } return StrHandler->emitLength(emitExpr(Call->getArgs()[0].get())); } if (Name == "MID") { - if (Call->getArgs().size() != 3) { reportError("MID expects 3 args"); return nullptr; } Value *Str = emitExpr(Call->getArgs()[0].get()); Value *Start = coerceValueToType(emitExpr(Call->getArgs()[1].get()), resolveType("INTEGER")); Value *Len = coerceValueToType(emitExpr(Call->getArgs()[2].get()), resolveType("INTEGER")); @@ -487,29 +522,24 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { return StrHandler->emitMid(Str, Start, Len); } if (Name == "RIGHT") { - if (Call->getArgs().size() != 2) { reportError("RIGHT expects 2 args"); return nullptr; } Value *Str = emitExpr(Call->getArgs()[0].get()); Value *Len = coerceValueToType(emitExpr(Call->getArgs()[1].get()), resolveType("INTEGER")); if (!Str || !Len) return nullptr; return StrHandler->emitRight(Str, Len); } if (Name == "LEFT") { - if (Call->getArgs().size() != 2) { reportError("LEFT expects 2 args"); return nullptr; } Value *Str = emitExpr(Call->getArgs()[0].get()); Value *Len = coerceValueToType(emitExpr(Call->getArgs()[1].get()), resolveType("INTEGER")); if (!Str || !Len) return nullptr; return StrHandler->emitLeft(Str, Len); } if (Name == "LCASE") { - if (Call->getArgs().size() != 1) { reportError("LCASE expects 1 arg"); return nullptr; } return StrHandler->emitLCase(emitExpr(Call->getArgs()[0].get())); } if (Name == "UCASE") { - if (Call->getArgs().size() != 1) { reportError("UCASE expects 1 arg"); return nullptr; } return StrHandler->emitUCase(emitExpr(Call->getArgs()[0].get())); } if (Name == "ASC") { - if (Call->getArgs().size() != 1) { reportError("ASC expects 1 arg"); return nullptr; } Value *ArgVal = emitExpr(Call->getArgs()[0].get()); const TypeInfo *ArgType = getExprTypeInfo(Call->getArgs()[0].get()); Value *CharVal = nullptr; @@ -522,7 +552,6 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { return coerceValueToType(AscVal, resolveType("INTEGER")); } if (Name == "CHR") { - if (Call->getArgs().size() != 1) { reportError("CHR expects 1 arg"); return nullptr; } Value *IntVal = coerceValueToType(emitExpr(Call->getArgs()[0].get()), resolveType("INTEGER")); Value *CharVal = ChrHandler->emitChr(IntVal); Function *MallocF = TheModule->getFunction("malloc"); @@ -535,11 +564,9 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { return Mem; } if (Name == "IS_NUM") { - if (Call->getArgs().size() != 1) { reportError("IS_NUM expects 1 arg"); return nullptr; } return StrConvHandler->emitIsNum(emitExpr(Call->getArgs()[0].get())); } if (Name == "NUM_TO_STR") { - if (Call->getArgs().size() != 1) { reportError("NUM_TO_STR expects 1 arg"); return nullptr; } Value *NumV = emitExpr(Call->getArgs()[0].get()); bool IsReal = NumV->getType()->isDoubleTy(); if (NumV->getType()->isIntegerTy(8)) { @@ -548,7 +575,6 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { return StrConvHandler->emitNumToStr(NumV, IsReal); } if (Name == "STR_TO_NUM") { - if (Call->getArgs().size() != 1) { reportError("STR_TO_NUM expects 1 arg"); return nullptr; } return StrConvHandler->emitStrToNum(emitExpr(Call->getArgs()[0].get()), true); } diff --git a/lib/Lexer/Lexer.cc b/lib/Lexer/Lexer.cc index d678097..a8af837 100644 --- a/lib/Lexer/Lexer.cc +++ b/lib/Lexer/Lexer.cc @@ -110,13 +110,6 @@ int Lexer::gettok() { if (IdentifierStr == "OR") return tok_or; if (IdentifierStr == "NOT") return tok_not; - if (IdentifierStr == "LENGTH") return tok_length; - if (IdentifierStr == "MID") return tok_mid; - if (IdentifierStr == "RIGHT") return tok_right; - if (IdentifierStr == "LEFT") return tok_left; - if (IdentifierStr == "LCASE") return tok_lcase; - if (IdentifierStr == "UCASE") return tok_ucase; - if (IdentifierStr == "FUNCTION") return tok_function; if (IdentifierStr == "ENDFUNCTION") return tok_endfunction; if (IdentifierStr == "PROCEDURE") return tok_procedure; diff --git a/lib/Parser/Parser.cc b/lib/Parser/Parser.cc index 049f99d..320fdf4 100644 --- a/lib/Parser/Parser.cc +++ b/lib/Parser/Parser.cc @@ -132,34 +132,6 @@ std::string Parser::ParseTypeName(bool AllowVoid) { return ""; } -std::unique_ptr Parser::ParseStringBuiltin(const std::string &FuncName) { - getNextToken(); - if (CurTok != '(') { - fprintf(stderr, "Error: Expected '(' after %s\n", FuncName.c_str()); - return nullptr; - } - 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 %s\n", FuncName.c_str()); - return nullptr; - } - getNextToken(); - } - } - getNextToken(); - return std::make_unique(FuncName, std::move(Args)); -} - std::unique_ptr Parser::ParseParenExpr() { getNextToken(); auto V = ParseExpression(); @@ -187,13 +159,6 @@ std::unique_ptr Parser::ParsePrimary() { getNextToken(); return Res; } - case tok_length: return ParseStringBuiltin("LENGTH"); - case tok_mid: return ParseStringBuiltin("MID"); - case tok_right: return ParseStringBuiltin("RIGHT"); - case tok_left: return ParseStringBuiltin("LEFT"); - case tok_lcase: return ParseStringBuiltin("LCASE"); - case tok_ucase: return ParseStringBuiltin("UCASE"); - case tok_true: { getNextToken(); return std::make_unique(true); -- cgit v1.3.1-4-g156e