diff --git a/compiler/genpc/genpc.go b/compiler/genpc/genpc.go index 49efb61..5b67627 100644 --- a/compiler/genpc/genpc.go +++ b/compiler/genpc/genpc.go @@ -291,17 +291,42 @@ func (g *generator) emitFor(s *ast.ForStmt) { if !ok { return } - // Init + // Init: var := start g.emitExpr(s.Start) g.emit(hbrt.PcOpPopLocal) g.emitU16(uint16(idx)) + // Detect step direction statically (matches gengo's emitFor): + // * no Step → +1, ascending + // * literal -N → descending + // * unary MINUS → descending + // Anything else (variable, expression) defaults to ascending. + // Without this we always emitted `var <= to`, which made `FOR + // 5 TO 1 STEP -1` exit on the first iteration; and we always + // stepped by hardcoded +1, which made `FOR i := 1 TO 10 STEP + // 2` summed 1+2+...+10 (55) instead of 1+3+5+7+9 (25). + negStep := false + if s.Step != nil { + if lit, ok := s.Step.(*ast.LiteralExpr); ok { + if lit.Kind == token.INT && len(lit.Value) > 0 && lit.Value[0] == '-' { + negStep = true + } + } + if un, ok := s.Step.(*ast.UnaryExpr); ok && un.Op == token.MINUS { + negStep = true + } + } + loopStart := g.pc() - // Check: var <= to + // Comparison: ascending → var <= to; descending → var >= to. g.emit(hbrt.PcOpPushLocal) g.emitU16(uint16(idx)) g.emitExpr(s.To) - g.emit(hbrt.PcOpLessEq) + if negStep { + g.emit(hbrt.PcOpGreaterEq) + } else { + g.emit(hbrt.PcOpLessEq) + } jumpOut := g.emitJumpPlaceholder(hbrt.PcOpJumpFalse) // Body @@ -309,30 +334,22 @@ func (g *generator) emitFor(s *ast.ForStmt) { g.emitStmt(stmt) } - // Step + // Increment: var := var + step (re-evaluating step per iter is + // fine; constant-folding can hoist it later). Push var, push + // step, add, store back. + g.emit(hbrt.PcOpPushLocal) + g.emitU16(uint16(idx)) if s.Step != nil { g.emitExpr(s.Step) } else { g.emit(hbrt.PcOpPushInt) g.emitI64(1) } - g.emit(hbrt.PcOpPushLocal) + g.emit(hbrt.PcOpPlus) + g.emit(hbrt.PcOpPopLocal) g.emitU16(uint16(idx)) - g.emit(hbrt.PcOpPlus) // swap order: step + local - // Actually need: local + step - // Fix: push local first, then step, then plus - // Let me redo: - // Undo the above and redo properly - g.code = g.code[:len(g.code)-1] // remove PcOpPlus - // Remove the PushLocal - g.code = g.code[:len(g.code)-3] - // Remove the step expr or PushInt - // This is getting complicated. Let me use LocalAddInt for simple step. - g.emit(hbrt.PcOpLocalAddInt) - g.emitU16(uint16(idx)) - g.emitI32(1) // default step = 1 - // Jump back + // Jump back to comparison g.emit(hbrt.PcOpJump) g.emitI32(int32(loopStart - g.pc() - 4)) @@ -557,6 +574,28 @@ func (g *generator) emitCallStmt(e *ast.CallExpr) { } func (g *generator) emitAssign(a *ast.AssignExpr) { + // Compound operators (+=, -=, *=, /=, %=, ^=) need to fold the + // existing left-hand value with the right. Without this they got + // emitted as plain `:=`, dropping the accumulator: `n += i` + // behaved as `n := i`. So the FOR loop reduce idiom (e.g. + // `n := 0 ; FOR i := 1 TO 10 ; n += i ; NEXT`) returned only + // the LAST iteration's increment. + if a.Op != token.ASSIGN { + op, ok := compoundBinOp(a.Op) + if ok { + if ident, isIdent := a.Left.(*ast.IdentExpr); isIdent { + if idx, found := g.locals[ident.Name]; found { + g.emit(hbrt.PcOpPushLocal) + g.emitU16(uint16(idx)) + g.emitExpr(a.Right) + g.emit(op) + g.emit(hbrt.PcOpPopLocal) + g.emitU16(uint16(idx)) + return + } + } + } + } if ident, ok := a.Left.(*ast.IdentExpr); ok { if idx, found := g.locals[ident.Name]; found { g.emitExpr(a.Right) @@ -577,6 +616,27 @@ func (g *generator) emitAssign(a *ast.AssignExpr) { g.emit(hbrt.PcOpPop) } +// compoundBinOp maps an `=` token to the binary opcode it +// produces against the left-hand value. Returns false for ASSIGN +// (the caller should take the plain-store path). +func compoundBinOp(k token.Kind) (byte, bool) { + switch k { + case token.PLUSEQ: + return hbrt.PcOpPlus, true + case token.MINUSEQ: + return hbrt.PcOpMinus, true + case token.STAREQ: + return hbrt.PcOpMult, true + case token.SLASHEQ: + return hbrt.PcOpDivide, true + case token.PERCENTEQ: + return hbrt.PcOpMod, true + case token.POWEREQ: + return hbrt.PcOpPower, true + } + return 0, false +} + func parseInt64(s string) int64 { var v int64 for _, c := range s { diff --git a/hbrt/frbmem.go b/hbrt/frbmem.go index 554efcc..ebd3bf9 100644 --- a/hbrt/frbmem.go +++ b/hbrt/frbmem.go @@ -24,14 +24,34 @@ import ( ) // FrbCompileSource compiles PRG source code to an FRB module in memory. -// If Go compiler is available, uses native plugin mode. -// If not, falls back to pcode interpreter mode (--pcode). +// Strategy: +// 1. If Go isn't installed, go straight to pcode mode. +// 2. Try the native Go-plugin path first (faster, native speed). +// 3. If the plugin build fails, OR if loading the resulting plugin +// fails (the most common failure: "plugin was built with a +// different version of package runtime", which fires whenever +// the host binary and the plugin weren't compiled byte-for-byte +// against the same Go runtime — happens routinely after `go +// build` rebuilds), fall back to pcode mode. Pcode is a few x +// slower but always works. func FrbCompileSource(vm *VM, prgSource string, fiveExe string) (*FrbModule, error) { - // Check if Go is available if !isGoAvailable() { return frbCompilePcode(vm, prgSource, fiveExe) } + mod, err := frbCompilePlugin(vm, prgSource, fiveExe) + if err == nil { + return mod, nil + } + // Plugin path failed — try pcode. Don't surface the plugin + // error: pcode either works (return its module) or doesn't + // (return its error). + return frbCompilePcode(vm, prgSource, fiveExe) +} + +// frbCompilePlugin is the original native-plugin path, factored out +// of FrbCompileSource so the latter can pick a fallback on failure. +func frbCompilePlugin(vm *VM, prgSource string, fiveExe string) (*FrbModule, error) { tmpDir, err := os.MkdirTemp("", "frb-mem-*") if err != nil { return nil, err diff --git a/hbrt/thread.go b/hbrt/thread.go index 386e8d5..3015160 100644 --- a/hbrt/thread.go +++ b/hbrt/thread.go @@ -219,6 +219,30 @@ func (t *Thread) Pop() { t.pop() } func (t *Thread) Pop2() Value { return t.pop() } // pop and return func (t *Thread) Dup() { t.push(t.peek()) } +// SP returns the current data-stack depth. Paired with SetSP for +// callers that need to clamp the stack across an inner function +// dispatch — used by FrbDo to neutralise pcode-body stack leaks. +func (t *Thread) SP() int { return t.sp } + +// SetSP forcibly resets the data-stack depth. Truncates if newSP < sp; +// extends with NIL if newSP > sp (defensive — should never grow here +// in practice). Bounds-checked against the underlying slice so a +// negative or out-of-range value can't corrupt the runtime. +func (t *Thread) SetSP(newSP int) { + if newSP < 0 { + newSP = 0 + } + if newSP > len(t.stack) { + newSP = len(t.stack) + } + // Clear any slots being abandoned so stale values can't surface + // later through Dup/peek paths. + for i := newSP; i < t.sp; i++ { + t.stack[i] = cachedNil + } + t.sp = newSP +} + // --- Frame management --- // Harbour: hb_xvmFrame(params, locals) // Called at the start of every function. diff --git a/hbrtl/frb.go b/hbrtl/frb.go index 27d1d55..181c298 100644 --- a/hbrtl/frb.go +++ b/hbrtl/frb.go @@ -14,6 +14,9 @@ package hbrtl import ( + "five/compiler/genpc" + "five/compiler/parser" + "five/compiler/pp" "five/hbrt" "fmt" "os" @@ -22,6 +25,65 @@ import ( "strings" ) +// frbCompileInProc compiles PRG source to a pcode FrbModule entirely +// in-process — no external `five` binary needed. Used by FrbCompile/ +// FrbExec when the host can't shell out (running from a directory +// where `five` isn't on PATH and isn't next to the binary). Avoids +// the plugin-runtime-mismatch failure mode of native FRB plugins +// AND removes the "find the five exe" fragility entirely. +func frbCompileInProc(vm *hbrt.VM, prgSource string) (*hbrt.FrbModule, error) { + prep := pp.New() + processed, errs := prep.Process("dynamic.prg", prgSource) + if len(errs) > 0 { + return nil, fmt.Errorf("preprocess: %s", strings.Join(errs, "; ")) + } + file, perrs := parser.Parse("dynamic.prg", processed) + if len(perrs) > 0 { + msgs := make([]string, 0, len(perrs)) + for _, e := range perrs { + msgs = append(msgs, e.Error()) + } + return nil, fmt.Errorf("parse: %s", strings.Join(msgs, "; ")) + } + pcMod := genpc.Generate(file) + + // Build a FrbModule from the pcode functions. Mirrors what + // hbrt/frb.go's frbLoadPcode does, but without the disk hop. + frbMod := &hbrt.FrbModule{ + Name: "dynamic", + LocalSyms: make(map[string]*hbrt.Symbol), + OldSyms: make(map[string]*hbrt.Symbol), + BindMode: hbrt.FrbBindDefault, + VM: vm, + } + for name, fn := range pcMod.Funcs { + pcFn := fn + pcModRef := pcMod + goFunc := func(t *hbrt.Thread) { + hbrt.ExecPcode(t, pcFn, pcModRef) + } + frbMod.LocalSyms[name] = &hbrt.Symbol{ + Name: name, + Scope: hbrt.FsPublic | hbrt.FsLocal, + Func: goFunc, + } + } + // Register non-Main symbols globally (Main stays module-local). + for name, sym := range frbMod.LocalSyms { + if name == "MAIN" { + continue + } + old := vm.FindSymbol(name) + if old != nil { + frbMod.OldSyms[name] = old + continue + } + vm.RegisterSymbol(sym) + frbMod.Registered = append(frbMod.Registered, name) + } + return frbMod, nil +} + // findFiveExe locates the 'five' compiler binary func findFiveExe() string { // 1. Check same directory as running executable @@ -86,6 +148,16 @@ func FrbDoFunc(t *hbrt.Thread) { return } + // Snapshot SP *before* pushing args. After the inner call, + // Frame()/PcOpRetValue should have left SP back at this baseline, + // but pcode-mode bodies can occasionally leak intermediate stack + // values (e.g. FOR-loop control vestiges). Reseating SP to the + // snapshot before reading retVal stops those leaks from polluting + // the caller's argument frame — which is what made + // `? "label", FrbDo(...), "tail"` show "1" or "2" in place of the + // label string when the inner function had a loop. + savedSP := t.SP() + // Push args for the function for i := 3; i <= nParams; i++ { t.PushValue(t.Local(i)) @@ -93,6 +165,7 @@ func FrbDoFunc(t *hbrt.Thread) { t.PendingParams2(nParams - 2) fn(t) + t.SetSP(savedSP) t.PushValue(t.GetRetValue()) t.RetValue() } @@ -112,14 +185,17 @@ func FrbUnloadFunc(t *hbrt.Thread) { } // FRBCOMPILE(cPrgSource) → pModule -// Compile PRG source string to FRB module in memory. +// Compile PRG source string to FRB module in memory. In-process pcode +// compilation is the default — no external `five` binary or `go` +// toolchain needed at runtime. The legacy native-plugin path is still +// reachable via hbrt.FrbCompileSource for callers that want it, but +// that path is fragile (Go plugins require byte-identical runtime). func FrbCompileFunc(t *hbrt.Thread) { t.Frame(1, 0) defer t.EndProc() source := t.Local(1).AsString() - fiveExe := findFiveExe() - mod, err := hbrt.FrbCompileSource(t.VM(), source, fiveExe) + mod, err := frbCompileInProc(t.VM(), source) if err != nil { fmt.Fprintf(os.Stderr, "FrbCompile error: %v\n", err) t.RetNil() @@ -136,8 +212,7 @@ func FrbExecFunc(t *hbrt.Thread) { defer t.EndProc() source := t.Local(1).AsString() - fiveExe := findFiveExe() - mod, err := hbrt.FrbCompileSource(t.VM(), source, fiveExe) + mod, err := frbCompileInProc(t.VM(), source) if err != nil { fmt.Fprintf(os.Stderr, "FrbExec error: %v\n", err) t.RetNil() @@ -145,18 +220,23 @@ func FrbExecFunc(t *hbrt.Thread) { } defer hbrt.FrbUnload(mod) - // Find and execute MAIN - sym := t.VM().FindSymbol("MAIN") - if sym == nil || sym.Func == nil { + // Look up MAIN inside the freshly-compiled module first, NOT + // via t.VM().FindSymbol — Main is intentionally kept module-local + // (frbLoadPcode skips it during VM registration), so a global + // lookup would resolve to the *caller's* Main and recurse forever. + fn := mod.FindFunc("MAIN") + if fn == nil { t.RetNil() return } + savedSP := t.SP() for i := 2; i <= nParams; i++ { t.PushValue(t.Local(i)) } t.PendingParams2(nParams - 1) - sym.Func(t) + fn(t) + t.SetSP(savedSP) t.PushValue(t.GetRetValue()) t.RetValue() @@ -177,19 +257,22 @@ func FrbRunFunc(t *hbrt.Thread) { } defer hbrt.FrbUnload(mod) - // Find MAIN symbol - sym := t.VM().FindSymbol("MAIN") - if sym == nil || sym.Func == nil { + // Same module-local Main lookup as FrbExec — see comment there + // for why a t.VM().FindSymbol("MAIN") would recurse into the + // outer (caller's) Main. + fn := mod.FindFunc("MAIN") + if fn == nil { t.RetNil() return } - // Push args + savedSP := t.SP() for i := 2; i <= nParams; i++ { t.PushValue(t.Local(i)) } t.PendingParams2(nParams - 1) - sym.Func(t) + fn(t) + t.SetSP(savedSP) t.PushValue(t.GetRetValue()) t.RetValue() diff --git a/tests/frb/frb_simple.prg b/tests/frb/frb_simple.prg new file mode 100644 index 0000000..8367dcf --- /dev/null +++ b/tests/frb/frb_simple.prg @@ -0,0 +1,12 @@ +FUNCTION GiveFive() + RETURN 5 + +FUNCTION AddOne(n) + RETURN n + 1 + +FUNCTION CountTo3() + LOCAL i, sum := 0 + FOR i := 1 TO 3 + sum := sum + i + NEXT + RETURN sum diff --git a/tests/frb/run.sh b/tests/frb/run.sh new file mode 100755 index 0000000..2507789 --- /dev/null +++ b/tests/frb/run.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash +# +# FRB regression runner. Each test exercises a different aspect of +# Five's runtime compilation / loading pipeline: +# +# frb_simple — fixture PRG built into a pcode FRB module. +# test_frb_simple — load `frb_simple.frb`, call its functions. +# test_frb_pcode_load — load mathlib (multi-function pcode FRB). +# test_frb_compile — FrbCompile / FrbExec — in-memory compile. +# test_frb_loop — FOR loop accumulators (`+=` and `:=`). +# test_frb_step — FOR ... STEP -1 / STEP 2 in pcode mode. +# +# Builds frb_simple.frb (and mathlib_pc.frb if needed) into the +# scratch dir before running the loaders. +set -e + +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +FIVE="$ROOT/five" +[ -x "$FIVE" ] || { echo "five not built — run 'go build -o five ./cmd/five'" >&2; exit 2; } + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +# Pre-build pcode FRB fixtures the loader tests refer to. +"$FIVE" frb "$ROOT/tests/frb/frb_simple.prg" -o /tmp/frb_simple.frb --pcode >/dev/null +"$FIVE" frb "$ROOT/examples/frb_mathlib.prg" -o /tmp/mathlib_pc.frb --pcode >/dev/null + +# Test files in the order they should run. test_frb_compile +# exercises the in-process compiler, which has no external +# dependencies — runs from any directory. +TESTS=( + test_frb_simple + test_frb_pcode_load + test_frb_compile + test_frb_loop + test_frb_step +) + +pass=0 +fail=0 +for name in "${TESTS[@]}"; do + src="$ROOT/tests/frb/${name}.prg" + bin="$work/${name}" + if ! "$FIVE" build "$src" -o "$bin" >/dev/null 2>"$work/${name}.err"; then + echo "FAIL build $name" + sed 's/^/ /' "$work/${name}.err" + fail=$((fail+1)) + continue + fi + if ! out="$("$bin" 2>&1)"; then + echo "FAIL run $name" + echo "$out" | sed 's/^/ /' + fail=$((fail+1)) + continue + fi + if echo "$out" | grep -qE 'FAIL|expect.*got|panic'; then + echo "FAIL assert $name" + echo "$out" | sed 's/^/ /' + fail=$((fail+1)) + continue + fi + echo "PASS $name" + pass=$((pass+1)) +done + +echo +echo "================================================================" +echo " Results: $pass / $((pass+fail)) passed" +echo "================================================================" +[ $fail -eq 0 ] diff --git a/tests/frb/test_frb_compile.prg b/tests/frb/test_frb_compile.prg new file mode 100644 index 0000000..6ea880c --- /dev/null +++ b/tests/frb/test_frb_compile.prg @@ -0,0 +1,38 @@ +/* In-memory PRG compilation via FrbCompile / FrbExec. */ + +FUNCTION Main() + LOCAL pStr, cSource + + /* 1. FrbCompile a small lib, call its functions */ + cSource := ; + 'FUNCTION Reverse(cStr)' + Chr(10) + ; + ' LOCAL i, cResult := ""' + Chr(10) + ; + ' FOR i := Len(cStr) TO 1 STEP -1' + Chr(10) + ; + ' cResult += SubStr(cStr, i, 1)' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN cResult' + Chr(10) + ; + 'FUNCTION Triple(n)' + Chr(10) + ; + ' RETURN n * 3' + Chr(10) + + pStr := FrbCompile(cSource) + IF pStr == NIL + ? "FAIL: FrbCompile returned NIL" + RETURN NIL + ENDIF + + ? "1. Reverse('Hello') =", FrbDo(pStr, "REVERSE", "Hello"), "(expect olleH)" + ? "2. Triple(7) =", FrbDo(pStr, "TRIPLE", 7), "(expect 21)" + FrbUnload(pStr) + + /* 2. One-shot FrbExec */ + ? "3. Sum 1..100 via FrbExec:" + ? " ", FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, n := 0' + Chr(10) + ; + ' FOR i := 1 TO 100' + Chr(10) + ; + ' n += i' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN n' + Chr(10) ), "(expect 5050)" + + ? "DONE" + RETURN NIL diff --git a/tests/frb/test_frb_loop.prg b/tests/frb/test_frb_loop.prg new file mode 100644 index 0000000..7e9be5b --- /dev/null +++ b/tests/frb/test_frb_loop.prg @@ -0,0 +1,33 @@ +FUNCTION Main() + LOCAL r + + /* in-memory compile a simple loop */ + r := FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, n := 0' + Chr(10) + ; + ' FOR i := 1 TO 10' + Chr(10) + ; + ' n := n + i' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN n' + Chr(10) ) + ? "sum 1..10 (using :=) =", r, "(expect 55)" + + r := FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, n := 0' + Chr(10) + ; + ' FOR i := 1 TO 10' + Chr(10) + ; + ' n += i' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN n' + Chr(10) ) + ? "sum 1..10 (using +=) =", r, "(expect 55)" + + /* string accumulator */ + r := FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, s := ""' + Chr(10) + ; + ' FOR i := 1 TO 5' + Chr(10) + ; + ' s := s + Str(i, 1)' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN s' + Chr(10) ) + ? "concat 1..5 =", r, '(expect "12345")' + + RETURN NIL diff --git a/tests/frb/test_frb_pcode_load.prg b/tests/frb/test_frb_pcode_load.prg new file mode 100644 index 0000000..c186cb0 --- /dev/null +++ b/tests/frb/test_frb_pcode_load.prg @@ -0,0 +1,20 @@ +/* Test loading + calling a pcode FRB module. */ + +FUNCTION Main() + LOCAL pMod + + pMod := FrbLoad("/tmp/mathlib_pc.frb") + IF pMod == NIL + ? "FAIL: FrbLoad returned NIL" + RETURN NIL + ENDIF + + ? "CircleArea(5.0) =", FrbDo(pMod, "CIRCLEAREA", 5.0), "(expect 78.539...)" + ? "Fibonacci(10) =", FrbDo(pMod, "FIBONACCI", 10), "(expect 55)" + ? "Fibonacci(20) =", FrbDo(pMod, "FIBONACCI", 20), "(expect 6765)" + ? "IsPrime(97) =", FrbDo(pMod, "ISPRIME", 97), "(expect .T.)" + ? "IsPrime(100) =", FrbDo(pMod, "ISPRIME", 100), "(expect .F.)" + + FrbUnload(pMod) + ? "DONE" + RETURN NIL diff --git a/tests/frb/test_frb_simple.prg b/tests/frb/test_frb_simple.prg new file mode 100644 index 0000000..c740097 --- /dev/null +++ b/tests/frb/test_frb_simple.prg @@ -0,0 +1,9 @@ +FUNCTION Main() + LOCAL pMod := FrbLoad("/tmp/frb_simple.frb") + + ? "A:", FrbDo(pMod, "GIVEFIVE"), "(expect 5)" + ? "B:", FrbDo(pMod, "ADDONE", 100), "(expect 101)" + ? "C:", FrbDo(pMod, "COUNTTO3"), "(expect 6)" + + FrbUnload(pMod) + RETURN NIL diff --git a/tests/frb/test_frb_step.prg b/tests/frb/test_frb_step.prg new file mode 100644 index 0000000..00bbf71 --- /dev/null +++ b/tests/frb/test_frb_step.prg @@ -0,0 +1,24 @@ +FUNCTION Main() + LOCAL r + + /* STEP -1 (downward) */ + r := FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, n := 0' + Chr(10) + ; + ' FOR i := 5 TO 1 STEP -1' + Chr(10) + ; + ' n := n + i' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN n' + Chr(10) ) + ? "FOR 5 TO 1 STEP -1 sum =", r, "(expect 15)" + + /* STEP 2 (upward by 2) */ + r := FrbExec( ; + 'FUNCTION Main()' + Chr(10) + ; + ' LOCAL i, n := 0' + Chr(10) + ; + ' FOR i := 1 TO 10 STEP 2' + Chr(10) + ; + ' n := n + i' + Chr(10) + ; + ' NEXT' + Chr(10) + ; + ' RETURN n' + Chr(10) ) + ? "FOR 1 TO 10 STEP 2 sum =", r, "(expect 25)" + + RETURN NIL