From 38ff4e782ef844a21512eb2ca0e491387ae89ec5 Mon Sep 17 00:00:00 2001 From: _ov4 Date: Tue, 13 Oct 2026 05:25:09 +0000 Subject: fix: compare strings by content Co-Authored-By: Claude Fable 5.1 --- AGENTS/CONVENTIONS.md | 7 +++++++ AGENTS/EXTENDING.md | 5 +++-- AGENTS/PITFALLS.md | 9 +++------ AGENTS/SUBSYSTEMS.md | 2 +- include/cps/FileHandler.h | 4 ++-- include/cps/StringHandler.h | 2 ++ lib/CodeGen/ArithmeticHandler.cc | 2 +- lib/CodeGen/CodeGen.cc | 15 +++++++++++++++ lib/CodeGen/StringHandler.cc | 22 ++++++++++++++++++++++ 9 files changed, 56 insertions(+), 12 deletions(-) diff --git a/AGENTS/CONVENTIONS.md b/AGENTS/CONVENTIONS.md index 6681d1a..e2c6ebd 100644 --- a/AGENTS/CONVENTIONS.md +++ b/AGENTS/CONVENTIONS.md @@ -57,6 +57,13 @@ CHAR = `i8`, STRING = opaque `ptr` to a NUL-terminated buffer. Some code still i pseudocode types *backwards* from LLVM types (`getExprTypeInfo`'s fallback, `ArithmeticHandler`'s `isIntegerTy(64)` tests), so these widths are ABI. +**Reference storage does not imply address comparison.** Compare values by content; +only explicitly declared pointer types compare addresses. STRING's six comparison +operators use `strcmp` in `StringHandler`, dispatched by pseudocode type in CodeGen +before ArithmeticHandler. Ordering is case-sensitive byte lexicographic order; +mixed STRING/non-STRING comparisons are errors. Whole-array and record comparisons +remain unsupported. + **Strings are malloc'd and never freed — by design.** Every string-producing operation (`&`, MID/LEFT/RIGHT/LCASE/UCASE, NUM_TO_STR, CHR, READFILE, CHAR/number→STRING coercion) mallocs a fresh buffer; string literals and BOOLEAN→STRING coercion instead diff --git a/AGENTS/EXTENDING.md b/AGENTS/EXTENDING.md index 0e12241..7a44281 100644 --- a/AGENTS/EXTENDING.md +++ b/AGENTS/EXTENDING.md @@ -25,8 +25,9 @@ branches do. Builtins are plain identifiers — nothing is needed in lexer/parse **New binary operator:** token (if multi-char) → one line in `BinopPrecedence` (Parser ctor; levels: OR=3, AND=5, comparisons=10, `+ - &`=20, `* / DIV MOD`=40; precedence must be > 0) → branch in `ArithmeticHandler::emitBinaryOp`, which dispatches -on raw LLVM operand types. Exception: `&` is intercepted upstream in `CodeGen::emitExpr` -(operands coerced to STRING, then `StringHandler::emitConcat`). +on raw LLVM operand types. Exceptions in `CodeGen::emitExpr`: `&` coerces operands +to STRING then calls `StringHandler::emitConcat`; STRING comparisons require two +STRING operands by pseudocode type then call `StringHandler::emitCompare`. **New builtin type:** register in `TypeSystem::registerBuiltins` (sizes are derived — never hand-written) + `TypeKind` case in the **exhaustive, default-less switches**: diff --git a/AGENTS/PITFALLS.md b/AGENTS/PITFALLS.md index acf21d2..6d1b196 100644 --- a/AGENTS/PITFALLS.md +++ b/AGENTS/PITFALLS.md @@ -5,8 +5,6 @@ Every bullet below was confirmed against the source, most also by compiling and running 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` 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 @@ -48,10 +46,9 @@ test programs. keywords now — `DECLARE READ : INTEGER` no longer compiles. `WRITEFILE f, a, b` takes exactly two operands (the stray `, b` is a parse error, like multi-expression OUTPUT). READFILE's target must be a plain STRING variable — no CHAR, no `arr[i]` (mirrors - INPUT). The CIE guide's empty-line test `IF LineOfText = ""` silently fails here - because STRING `=` is pointer identity — use `LENGTH(LineOfText) = 0`. At most 16 - files open at once. All runtime misuse (not open, wrong mode, reopen, fopen failure, - READFILE past EOF) is a `[Fatal] line %d:` exit(1), by design — guard reads with + INPUT). The CIE guide's empty-line test `IF LineOfText = ""` compares contents. + At most 16 files open at once. All runtime misuse (not open, wrong mode, reopen, + fopen failure, READFILE past EOF) is a `[Fatal] line %d:` exit(1), by design — guard reads with `WHILE NOT EOF(...)`. Caveat: the open-file table is keyed by the filename *string* (per CIE semantics), so two spellings of one OS path (`"a.txt"` vs `"./a.txt"`) evade the already-open check and get two independent buffered streams — reads through the diff --git a/AGENTS/SUBSYSTEMS.md b/AGENTS/SUBSYSTEMS.md index 9f243c2..0075222 100644 --- a/AGENTS/SUBSYSTEMS.md +++ b/AGENTS/SUBSYSTEMS.md @@ -67,7 +67,7 @@ line. On first file use, `ensureRuntime()` synthesizes the whole runtime once only module-level helper functions in the project: internal-linkage `__cps_file_find/ open/read/write/close/eof` plus a 16-slot open-file table (`__cps_file_names/handles/ modes` parallel globals; mode byte 0 = free). Files are keyed by filename *content* -(strcmp scan) because STRING `=` is pointer identity; the name pointer is stored as-is +(strcmp scan), consistent with STRING comparison; the name pointer is stored as-is (strings are immutable and never freed). EOF is an fgetc/ungetc peek; READFILE grows a malloc/realloc buffer per line and strips one trailing `\r`; fopen modes are text `"r"/"w"/"a"` (Windows CRT translates CRLF). File-free programs get byte-identical IR — diff --git a/include/cps/FileHandler.h b/include/cps/FileHandler.h index aaad6a0..3898a57 100644 --- a/include/cps/FileHandler.h +++ b/include/cps/FileHandler.h @@ -11,8 +11,8 @@ class RuntimeCheck; // Emits the text-file runtime: a fixed-size open-file table plus internal // __cps_file_* helper functions, synthesized once on first use so programs // without file statements keep their IR unchanged. The table is keyed by -// filename *content* (strcmp) — pseudocode STRING equality is pointer -// identity, so the same file may be named by many different buffers. +// filename *content* (strcmp), so the same file may be named by many +// different buffers. // Leaf module: methods only take and return llvm::Value*s; expression // evaluation, type coercion and the symbol table stay in CodeGen. class FileHandler { diff --git a/include/cps/StringHandler.h b/include/cps/StringHandler.h index e9a8752..0b2447c 100644 --- a/include/cps/StringHandler.h +++ b/include/cps/StringHandler.h @@ -16,6 +16,7 @@ class StringHandler { llvm::FunctionCallee MemCpyFunc; llvm::FunctionCallee StrCpyFunc; llvm::FunctionCallee StrCatFunc; + llvm::FunctionCallee StrCmpFunc; llvm::FunctionCallee ToUpperFunc; llvm::FunctionCallee ToLowerFunc; @@ -30,6 +31,7 @@ public: llvm::Value *emitLength(llvm::Value *Str); llvm::Value *emitConcat(llvm::Value *LHS, llvm::Value *RHS); + llvm::Value *emitCompare(int Op, llvm::Value *LHS, llvm::Value *RHS); llvm::Value *emitMid(llvm::Value *Str, llvm::Value *Start, llvm::Value *Len); llvm::Value *emitRight(llvm::Value *Str, llvm::Value *Len); llvm::Value *emitLeft(llvm::Value *Str, llvm::Value *Len); diff --git a/lib/CodeGen/ArithmeticHandler.cc b/lib/CodeGen/ArithmeticHandler.cc index 439e4b1..fa71e0c 100644 --- a/lib/CodeGen/ArithmeticHandler.cc +++ b/lib/CodeGen/ArithmeticHandler.cc @@ -83,7 +83,7 @@ Value *ArithmeticHandler::emitBinaryOp(int Op, Value *LHS, Value *RHS, int Line) case tok_ge: return Builder.CreateFCmpOGE(LVal, RVal, "sgetmp"); } } else { - // Same-type pairs (i64/i64, i8/i8 CHAR, i1/i1, ptr/ptr STRING) go + // Same-type pairs (i64/i64, i8/i8 CHAR, i1/i1) go // through untouched; mixed integer widths are widened, anything else // mismatched is a diagnostic instead of invalid IR. Value *L = LHS; diff --git a/lib/CodeGen/CodeGen.cc b/lib/CodeGen/CodeGen.cc index 903d8ef..a6669a6 100644 --- a/lib/CodeGen/CodeGen.cc +++ b/lib/CodeGen/CodeGen.cc @@ -637,6 +637,21 @@ Value *CodeGen::emitExpr(ExprAST *Expr) { if (!L || !R) return nullptr; return StrHandler->emitConcat(L, R); } + // STRING and explicit pointers share LLVM's ptr representation; + // string comparisons must dispatch on the pseudocode type. + switch (Bin->getOp()) { + case tok_eq: case tok_ne: case '<': case '>': case tok_le: case tok_ge: + if ((LT && LT->isString()) || (RT && RT->isString())) { + if (!(LT && LT->isString() && RT && RT->isString())) { + reportError("STRING comparison requires two STRING operands (line %d)", Bin->getLine()); + return nullptr; + } + return StrHandler->emitCompare(Bin->getOp(), L, R); + } + break; + default: + break; + } return ArithHandler->emitBinaryOp(Bin->getOp(), L, R, Bin->getLine()); } diff --git a/lib/CodeGen/StringHandler.cc b/lib/CodeGen/StringHandler.cc index 546506d..0830021 100644 --- a/lib/CodeGen/StringHandler.cc +++ b/lib/CodeGen/StringHandler.cc @@ -1,4 +1,5 @@ #include "cps/StringHandler.h" +#include "cps/Lexer.h" #include "llvm/IR/Type.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Constants.h" @@ -27,6 +28,9 @@ void StringHandler::setupExternalFunctions() { false); StrCpyFunc = Module.getOrInsertFunction("strcpy", StrCpyType); StrCatFunc = Module.getOrInsertFunction("strcat", StrCpyType); + StrCmpFunc = Module.getOrInsertFunction("strcmp", + FunctionType::get(Type::getInt32Ty(Context), + {PointerType::getUnqual(Context), PointerType::getUnqual(Context)}, false)); FunctionType *CharCaseType = FunctionType::get(Type::getInt32Ty(Context), {Type::getInt32Ty(Context)}, false); ToUpperFunc = Module.getOrInsertFunction("toupper", CharCaseType); @@ -58,6 +62,24 @@ Value *StringHandler::emitConcat(Value *LHS, Value *RHS) { return NewStr; } +Value *StringHandler::emitCompare(int Op, Value *LHS, Value *RHS) { + if (!LHS || !RHS) return nullptr; + + CmpInst::Predicate Pred; + switch (Op) { + case tok_eq: Pred = CmpInst::ICMP_EQ; break; + case tok_ne: Pred = CmpInst::ICMP_NE; break; + case '<': Pred = CmpInst::ICMP_SLT; break; + case '>': Pred = CmpInst::ICMP_SGT; break; + case tok_le: Pred = CmpInst::ICMP_SLE; break; + case tok_ge: Pred = CmpInst::ICMP_SGE; break; + default: return nullptr; + } + + Value *Cmp = Builder.CreateCall(StrCmpFunc, {LHS, RHS}, "strcmp_result"); + return Builder.CreateICmp(Pred, Cmp, ConstantInt::get(Type::getInt32Ty(Context), 0), "str_compare"); +} + // malloc(Len + 1), copy Len bytes from Src + StartIdx, NUL-terminate. // Clamping is each caller's business — MID/RIGHT/LEFT clamp different things. Value *StringHandler::allocCopySubstring(Value *Src, Value *StartIdx, Value *Len) { -- cgit v1.3.1-4-g156e