Compiling _FiveSql2/test/test_sql_extreme.prg + a sweep of the FRB
demos surfaced four real bugs in the dynamic-compilation pipeline.
All fixes shipped together because they were on the same critical
path; each is independently revertible.
* **pcode FOR loop ignored STEP and direction.** emitFor in
compiler/genpc emitted a fixed `<= to` comparison and a hardcoded
`+1` increment, then deleted the actual step expression with
slice arithmetic on the byte buffer. Result: `FOR 5 TO 1 STEP
-1` exited on the first iteration; `FOR 1 TO 10 STEP 2` summed
1..10 (55) instead of 1+3+5+7+9 (25). Rewritten to mirror
gengo's emitFor: detect negative step from a literal `-N` or
unary MINUS, pick `<=` vs `>=` accordingly, and emit a clean
`var := var + step` increment per iteration.
* **pcode compound `+=` operator stored only the RHS.** emitAssign
looked at AssignExpr.Op only for the := case; +=/-=/etc.
silently took the same path, so `n += i` compiled as `n := i`,
discarding the accumulator. Loop reduces were wrong: `Reverse`
returned "" and `n := 0; FOR i ... n += i; NEXT` returned only
the last increment. New compoundBinOp helper maps PLUSEQ /
MINUSEQ / STAREQ / SLASHEQ / PERCENTEQ / POWEREQ to their
matching binary opcode; emitAssign emits `local + rhs ; pop
local` for compound forms.
* **Pcode body stack leaks polluted the caller's frame.** A pcode
function whose body left intermediate values on the data stack
(FOR control values, etc.) returned with extra entries past
its declared retVal. FrbDoFunc / FrbExecFunc / FrbRunFunc then
pushed retVal on top of those leaks, so the caller saw the
leaked values where its own preceding arguments should have
been: `? "Fibonacci(10) =", FrbDo(...), "(expect 55)"` printed
`1 55 (expect 55)` because the FOR loop's `1` lived in arg-1's
slot. Two new Thread methods (`SP()` / `SetSP(int)`) let the
three FRB dispatchers snapshot stack depth before the inner
call and clamp it back afterward, so the leaks evaporate before
they reach the caller's frame.
* **FrbExec / FrbRun recursed into the host's Main forever.** Both
looked up "MAIN" via t.VM().FindSymbol, which always resolved
to the OUTER program's Main since FRB modules deliberately keep
Main local. Compile + run + unload became compile + recurse +
OOM. Both now look up Main via mod.FindFunc("MAIN") (module
scope) — Frbload's policy of leaving Main module-local now
actually has the intended effect.
Plus an architectural improvement: in-memory compilation no longer
depends on shelling out to an external `five` binary. New
hbrtl.frbCompileInProc parses + preprocesses + generates pcode in
process, building a FrbModule directly. FrbCompile and FrbExec use
this exclusively, which means dynamic compilation works from any
directory regardless of PATH and without a second process. The
plugin-mode path (with its runtime-version-mismatch fragility) is
left available via hbrt.FrbCompileSource for callers that want it,
but FrbCompile no longer reaches for it by default.
Test suite: tests/frb/ holds five fixtures + a runner. 5/5 pass:
test_frb_simple / test_frb_pcode_load / test_frb_compile /
test_frb_loop / test_frb_step.
Other gates green:
go test ./... : PASS
FiveSql2 SQL:1999 : 43/43
Harbour compat : 56/56
std.ch suite : 14/14
FRB suite : 5/5
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six audit-driven blockers landed together because they're tangled:
* MENU TO removed from std.ch — the rule expanded to a call to a
nonexistent __MenuTo() RTL symbol, so any user code with `MENU
TO choice` compiled clean and panicked at runtime. Behavior
pre-this-round was a parser silent no-op, which is at least
consistent. Restore that until @ PROMPT (the companion command)
actually lands.
* COUNT now requires `TO <var>`. The earlier `[TO <v>]` optional
bracket was a Harbour-pattern transcription error: the result
template references `<v>` unconditionally, so a bare `COUNT`
expanded to ungrammatical ` := 0 ; dbEval(...)` and the
PRG parser rejected it. Match Harbour's std.ch which makes TO
mandatory.
* UPDATE FROM ... REPLACE now requires `FROM`/`ON`/`REPLACE` all
three. Same root cause as COUNT: the result template uses
`<key>`, `<f1>`, `<x1>` unconditionally; missing any of them
produced broken syntax. Tightened to fail loudly rather than
silently mis-expand.
* CLOSE <unknown_alias> no longer closes the *current* workarea.
SelectByAlias was a silent no-op when the alias was missing,
leaving WASaveAndSelectAlias to evaluate the inner DbCloseArea()
against the originally-selected WA — a real data-loss footgun.
SelectByAlias now returns bool; WASaveAndSelectAlias switches to
the no-area sentinel (0) on miss so the inner expression's
Current() returns nil and short-circuits.
* SUM <x1>, <xN> TO <v1>, <vN> — multi-pair form supported.
Required two pieces:
1. matchSegment's regular-marker stop-boundary now combines
outerTail literals AND the segment's repeat boundary so
`[, <xN>]` doesn't let `<xN>` swallow past the next ','.
2. **Five parser miscompiled comma-separated expressions in
code blocks.** `{|| e1, e2, e3 }` kept only the last expr
and threw away earlier ones at *AST level*, so all their
side effects vanished. New SeqExpr AST node + emitter
(emit each, pop intermediate results) + folding/walk
updates fix the underlying bug, which also unbreaks any
other block that relied on comma sequencing.
* pp.go's `;` continuation joiner now strips exactly one trailing
`;` per iteration, preserving Harbour's `;;` convention (literal
`;` followed by a continuation marker). Without this the SUM
rule's chained `<v1> :=[ <vN> :=] 0 ; ; dbEval(...)` collapsed
to a missing statement separator.
* parseExprStmt's xBase fallback switch is back in sync with
parseIdentStmt — COPY/SORT/COUNT/SUM/AVERAGE/TOTAL/UPDATE/JOIN/
DISPLAY/LIST removed (std.ch handles all of them now). Leaving
them in the fallback masked typos as silent no-ops.
Gates green:
go test ./... : PASS
FiveSql2 SQL:1999 : 43/43
Harbour compat : 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Opus 4.7 audit of the codebase surfaced several items that Opus 4.6
sessions left behind. This pass removes what's definitively dead and
fixes one trivial defensive bug; the real logic bugs (transaction
ordering, missing RunUpdate/RunDelete validation) come in a separate
commit.
Deletions:
- `_FiveSql2/src/TSqlParser_orig.prg` (1173 lines) — superseded by
`TSqlParser2.prg` (Pratt). Production never instantiates the old
parser; the only callers were the comparison/benchmark test files
also being removed.
- `_FiveSql2/test/test_parser_cmp.prg` — compared orig vs Pratt AST,
useless now that orig is gone.
- `_FiveSql2/test/bench_parser.prg` — benched both, same reason.
- `_FiveSql2/Makefile` `test_cmp:` and `bench:` targets referenced
the removed files.
- `TSqlIndex.prg` methods `ApplyScope`, `ClearScope`, `ApplySeek`,
`IndexInfo`, `CreateTempIndex`, `DropTempIndex` — each declared in
the class header and implemented (~165 lines total) but zero
callers anywhere in `_FiveSql2/` or `hbrtl/`. Class declarations
removed alongside the bodies.
Small fixes:
- `TSqlDDL.prg:179-180` stale comment claiming Five doesn't support
`@byref` — false since commit e95afad (2026-04-13) wired @byref
via RefCell. The same method uses @nPos correctly elsewhere.
- `hbrt/class.go:tryBinaryOp` defensive nil-check on AsArray().
IsObject() checks the type tag; a corrupted Value with tag=Object
but ptr=nil would crash on `.Class`. Correct construction paths
never hit this, but the guard is cheap.
Compat tests: FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Harbour's macro operator was a stub: hbrt.MacroCompile only resolved
bare identifier names to memvars/functions and returned the source
string unchanged for any non-trivial expression. The gengo emit was
also broken — `t.MacroPush() + t.PushNil()` never pushed the inner
expression's value, so MacroPush popped whatever happened to be on
the stack.
Wire it up properly:
1. Gengo fix: `case *ast.MacroExpr` now emits `emitExpr(e.Expr);
t.MacroPush()`. The inner expression produces the source string;
MacroPush consumes it and pushes the evaluated result.
2. Hook pattern in hbrt: `SetMacroEvalHook(fn)` lets hbrtl install
the real evaluator without creating an import cycle (genpc
already imports hbrt). MacroPush delegates to the hook when
installed; otherwise falls back to the legacy stub for hbrt
unit tests.
3. hbrtl.init registers macroEval, which reuses compileExprSource
(factored out of PcCompile) so macro lookups share the same
sync.Map-backed pcode cache — repeat evaluations of the same
macro source are free after the first hit.
4. ExecPcode leaves the result in retVal; macroEval copies it to
the operand stack via PushRetValue.
Tested (/tmp/test_macro.prg):
&"10 + 20" → 30
&"Sqrt(16)" → 4
&"Upper('hello')" → HELLO
&("30 * " + Str(nX, 1)) → 210 (runtime-built source)
&"5 > 3 .AND. .T." → .T.
&("Str(" + Str(nX*10,2) + ",2)") → 70
FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Harbour lets a class define custom behaviour for arithmetic and
comparison operators via `OPERATOR "<sym>" ARG <name> INLINE <expr>`.
Five already had the runtime slot infrastructure (ClassDef.Operators
+ AddOperator + parent-chain copy) but parser skipped the form and
the VM ops never consulted the slots.
Parser: parseOperatorDecl captures the symbol, ARG binding, and
INLINE body into a MethodDecl with IsOperator=true and OperatorOp
set to the hbrt.Op* slot. Synthesised method name is __OP_<idx>
to keep the regular method namespace clean.
Codegen: emitClassDecl routes IsOperator members through
_def.AddOperator instead of AddMethod. Inline body generation is
shared with the MESSAGE/INLINE path (34485cd).
VM: Thread.tryBinaryOp walks the LHS object's class operator slot,
pushes args with Self bound to LHS, and returns true if the slot
is populated. Wired into Plus/Minus/Mult/Divide and Equal/NotEqual/
Less/Greater/LessEqual/GreaterEqual. Falls through to built-in
behaviour when no overload exists — non-object LHS costs one tag
check per op.
Operator symbol→slot mapping keeps `=` and `==` on the same slot
(OpEqual=8) because Five's gengo routes both to t.Equal() and the
VM doesn't distinguish strict vs non-strict equality today.
Tested (/tmp/test_operator.prg): Vec2 + - == < with per-field
results all correct.
FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Harbour's ::super: idiom routes a method call through the parent of
the class that defines the currently-executing method — Self stays
the child instance, only the vtable entry point shifts. Five
previously parsed ::super as a data-field access (PushSelfField("SUPER"))
which returned nil and panicked on the subsequent Send.
Runtime: Thread.SendSuper(fromClassName, methodName, nArgs).
Binding to the *defining* class (not Self's runtime class) is
load-bearing for 3+ level hierarchies: without it,
Grand:New → ::super:New → Child:New → ::super:New
would resolve to Grand.Parent=Child again and infinite-loop.
Gengo: Generator.curMethodClass tracks the class name across each
method body emission. emitSendExpr detects the nested SendExpr
shape `::super:X(...)` and emits SendSuper with curMethodClass as
the first argument.
Tested (/tmp/test_super, /tmp/test_super2):
Parent → Child: ::super:Greet() returns composed result
Base → Child → Grand: ::super:New chain passes args correctly
Also fixes three gengo unit tests whose expected output was stale
from prior perf commits (b829ed4 const prop, 1f63c7f symbol hoist,
7e4079f string-concat reassoc) — assertions now match the current
optimized codegen.
FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply the sp-rewrite shape to the three binary arithmetic ops. The
tInt==tInt fast branch reads scalar directly (skips the AsNumInt
method) so the hot path is int64 ops + an overflow check; mixed-type
branches keep AsNumDouble unchanged.
PRG tight loops (FOR counter, SUM accumulators outside SQL aggregate
path) skip one cachedNil store and two bounds-check sequences per op.
Verification
- go test ./... ALL PASS
- FiveSql2 test_sql1999 43/43
- tests/compat_harbour 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fold And/Or into the same in-place sp-rewrite shape as Not/LessEqual.
Both args must be tLogical — short-circuit on the raw scalar field so
the hot path is pure integer arithmetic + two cached bool Values.
Verification
- go test ./... ALL PASS
- FiveSql2 test_sql1999 43/43
- tests/compat_harbour 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the treatment LessEqual already got onto Equal/NotEqual/Less/
Greater/GreaterEqual — rewrite sp directly, check the type tag for
Int==Int in the hot branch, short-circuit to cachedTrue/cachedFalse
without a second method call. Keeps the slow fallback for mixed /
string / date types.
Bench movement is minor on SQL paths (WHERE is already pcode and
skips these ops); the win is on PRG comparisons that cache-miss out
of the pcode path — FOR-condition short forms, IF chains, etc.
Verification
- go test ./... ALL PASS
- FiveSql2 test_sql1999 43/43
- tests/compat_harbour 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small tweaks to the pop-push hotspots in the VM.
- ops_arith.go Inc/Dec/AddInt: unary ops mutate the top stack slot in
place via peekPtr() instead of pop-compute-push. Drops the bounds
check + cachedNil clear + push bounds check per call. Biggest
beneficiary: FOR loop counters (implicit Inc) — every iteration of
every PRG loop pays these ops once.
- ops_collection.go ArrayGen: consume N slots via a single `copy`
into the freshly-allocated result slice, then rewind sp and clear
the intermediate slots for GC (the first slot is overwritten by
the array push). Skips the N-deep pop loop.
- ops_collection.go EvalBlock: read block value before shift, collapse
args down one slot to overwrite the block position, then let the
block run against the same in-place layout. Matches the
Function()/PushSymbol round-trip removal from the prior commit.
bench_sql deltas
- B2 WHERE 83 → 78 µs (6%)
- B3 ORDER BY 96 → 90 µs (6%)
- B4 GROUP_HAVING 554 → 528 µs (5%)
- B9 ROW_NUMBER 255 → 241 µs (5%)
- B10 RANK PART 296 → 278 µs (6%)
- B11 SUM OVER 320 → 300 µs (6%)
- B15 CTE+WIN+JOIN 1826 →1743 µs (5%)
Verification
- go test ./... ALL PASS
- FiveSql2 test_sql1999 43/43
- tests/compat_harbour 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The VM call path (PushSymbol → Function → Frame) is traversed by every
PRG function call. Three changes together cut per-call overhead across
the entire bench suite.
Changes
- hbrt/call.go Function(): replace pop-push dance with a single slice
shift (N+2 pops + N pushes → 1 copy of N slots + sp adjust). Kills
the per-call `make([]Value, nArgs)` heap alloc. Resolved function
pointer is cached back into sym.Func so subsequent calls on the
same Symbol skip the VM lookup entirely.
- hbrt/vm.go GetSym(): new helper. Generated code calls it with a
pointer to a package-level `*Symbol` slot so FindSymbol (which takes
the VM RWMutex + map lookup) runs at most once per symbol per
process. Nil results are intentionally NOT cached — an init-order
miss becomes a retry on the next call instead of a permanent sticky
failure.
- hbrt/thread.go pushPendingSym(): scalar fast slot for depth=1 call
nesting (common case). Nil syms still go through the slice so the
"empty vs stored nil" ambiguity can't produce a false pop.
- compiler/gengo/gengo.go: emit `t.PushSymbol(t.GetSym(&_sym_<file>_<NAME>, "NAME"))`
for every function call site, with a per-file prefix so multi-PRG
builds don't collide on identical symbol names.
Bugs fixed during bring-up
- pendingSymFast == nil was ambiguous ("unused" vs "nil stored"). Nil
syms now spill to the slice, preserving distinguishability.
- The old varName-reuse branch at the PushSymbol emit site skipped
the GetSym wrapper, emitting a raw `t.PushSymbol(varName)` against
an uninitialized package-level *Symbol. Every call path now funnels
through emitPushSymbol.
bench_sql deltas vs prior build
- B1 SELECT * 114 → 97 µs (15%)
- B4 GROUP_HAVING 584 → 554 µs (5%)
- B8 RECURSIVE CTE 150 → 141 µs (6%)
- B10 RANK PARTITION 310 → 296 µs (5%)
- B11 SUM OVER 335 → 320 µs (4%)
- B14 COUNT 295 → 281 µs (5%)
- B15 CTE+WIN+JOIN 1891 → 1826 µs (3%)
Verification
- go test ./... ALL PASS
- FiveSql2 test_sql1999 43/43
- tests/compat_harbour 56/56
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second pcode peephole to match the one added for FieldGet(literal).
SqlExprToPrg auto-wraps CHAR column references with AllTrim() to
match SqlCmpEq's CHAR-padding trim semantics, so every string WHERE
predicate evaluates `AllTrim(FieldGet(n)) == 'literal'` per row.
Before this commit each of those per-row evaluations did:
1. PushSymbol ALLTRIM
2. PushSymbol FIELDGET → Function(1) [1 RTL Frame]
3. parseCharField → MakeString [alloc: copies raw bytes]
4. Function(1) → AllTrim RTL [1 RTL Frame]
5. strings.TrimSpace [alloc: new string]
6. Return, continue
New opcode `PcOpFieldTrim <idx>` (0x47) fuses the two RTL calls into
a single opcode that:
1. Calls FastFieldGetter directly (no Frame/Function dispatch).
2. Walks the returned string with ASCII-space trim in place.
3. Pushes `s[lo:hi]` — a sub-slice, no new allocation.
4. Short-circuits back to the same string if no trim needed.
genpc recognizes the shape `AllTrim(FieldGet(<int-literal>))` in
emitCall and emits the fused opcode automatically — no SQL-side
API change. Matches the existing FieldGet peephole's shape.
Bench impact (50k rows, 3-run steady state, vs raw RDD baseline 6.2ms):
String WHERE before 7.9ms → after 6.2ms 1.00x (parity!)
Numeric WHERE 6.9ms (unchanged) 1.11x
No WHERE 9.1ms (unchanged) 1.47x
String WHERE is now at parity with the raw Harbour-style RDD scan.
Compared to session start (119ms), that's a 19x speedup.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two stacked optimizations land on the SqlScan hot path. Combined
effect on the 50k-row benchmark:
Before After vs raw
Numeric WHERE 10.2ms 7.8ms 1.15x
String WHERE 10.5ms 7.9ms 1.15x
No WHERE 9.2ms 10.0ms 1.45x
Raw RDD baseline 6.8ms 6.8ms 1.00x
WHERE-predicate paths are now within 15% of the raw Harbour-style
RDD scan loop. The no-WHERE path is unchanged (slight jitter from
the added devirt branch); FieldGet peephole doesn't apply there.
--- Optimization 1: PcOpFieldGet peephole ---
Adds a new pcode opcode `PcOpFieldGet <fieldIdx>` (0x46) that skips
the usual PushSymbol+Function+Frame+FieldGet-RTL+EndProc chain and
calls a direct field getter closure instead. genpc recognizes the
shape `FieldGet(<int-literal>)` during emitCall and emits the
specialized opcode automatically — no SQL-side API change.
Integration:
* hbrt.Thread.FastFieldGetter — hot-path closure set by scan loops.
Non-nil → pcode bypasses dispatch.
Nil → pcode resolves FIELDGET via
the RTL symbol table (correctness
fallback for any other callers).
* compiler/genpc/genpc.go — peephole in emitCall.
* hbrt/pcinterp.go — PcOpFieldGet handler.
This alone cut numeric WHERE from 10.2 → 7.9ms: eliminated roughly
one full Frame/EndProc + RTL dispatch per row × 50k rows.
--- Optimization 2: DBFArea devirtualization ---
SqlScan type-asserts the workarea to *dbf.DBFArea once and runs a
dedicated loop that calls GoTop/EOF/Skip/GetValue directly on the
concrete type. Go's compiler inlines these, skipping the interface
vtable per row. Non-DBF drivers still work via the generic Area
branch.
The FastFieldGetter closure also captures *DBFArea directly in the
DBF branch, so the WHERE predicate side of the hot loop is now
entirely devirtualized: no interface dispatch between the pcode
dispatch loop and the DBF record buffer.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Remaining gap to raw RDD on no-WHERE (~1.45x) is dominated by the
two-column row construction + ArraySlab + flat backing bookkeeping
that the raw loop doesn't do. Going below that requires changing
the SQL engine's result shape — out of scope here.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SqlScan's prior design called hbrt.MakeArrayFrom per matching row,
each one allocating a fresh &HbArray{}. For 50k rows that's 50k tiny
Go heap allocations + GC pressure that the flat-backing-buffer work
from 85541a3 left untouched (that commit eliminated the per-row items
slice alloc but not the header alloc).
hbrt.ArraySlab pre-allocates a `[]HbArray` slab of the estimated row
count and hands out `&slab.buf[idx]` on each WrapNext. One underlying
make() replaces N; pointers stay stable because slab growth reallocates
a fresh buffer instead of reusing the old one, so previously-handed-out
pointers remain valid (the old backing is kept alive by the references).
API kept tiny:
slab := hbrt.NewArraySlab(estRows)
val := slab.WrapNext(items) // returns Value wrapping &slab.buf[i]
SqlScan now pairs this with the existing flat value buffer for a
single-allocation-per-chunk scan hot loop.
Combined bench impact (50k rows, steady state):
Session start Now
no WHERE 14.6ms 9.2ms ← 1.3x vs raw RDD baseline
numeric WHERE 11.7ms 10.2ms
string WHERE 10.5ms 10.5ms
raw RDD baseline 6.8ms 7.0ms
no WHERE is now within 30% of raw RDD. Remaining gap is largely
Area.GetValue boxing overhead and the pcode opcode dispatch loop
itself — no further structural wins without a wider refactor.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pcode expressions compiled from SQL WHERE clauses (via genpc.CompileExpr)
never contain BEGIN SEQUENCE and can't raise BreakValue, so the defer +
recover dance in ExecPcode's EndProc is pure overhead. For FiveSql2's
per-row WHERE evaluation on a 50k-row scan, that's 50k × ~15ns = ~750µs
of pointless recover bookkeeping.
Split ExecPcode into two variants sharing execPcodeBody:
ExecPcode — full: Frame + defer EndProc. General-purpose,
handles panics. Behavior unchanged.
ExecPcodeFast — hot: Frame + execPcodeBody + EndProcFast. No defer,
no recover. Caller guarantees the pcode body can't
panic with HbError / BreakValue.
SqlScan now uses ExecPcodeFast for per-row WHERE evaluation. Measured
impact on 50k-row no-WHERE benchmark: 10.6ms → 9.2ms steady state
(~13% faster). Effect is smaller on numeric-WHERE because per-row
cost there is dominated by the opcode dispatch itself, not the frame
exit.
Validation:
- FiveSql2 43/43
- go test ./hbrt/... PASS (pcode tests)
- go test ./hbrtl/... PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Release-blocking compatibility issues discovered during the 258-test
pre-release validation suite (100 syntax + 44 RDD + 114 RTL).
1. PCount() always returned 0 in PRG code
Root cause: ParamCount() returned t.pendingParams, which is
overwritten by every nested Function() call. By the time the
PCount() RTL's Frame() executes, pendingParams is already 0.
Fix: Frame() now stores pendingParams in frame.paramCount.
PCount() RTL uses CallerParamCount() which reads callSP-2
(the PRG caller's frame), while RTL functions still use
ParamCount() (reads pendingParams before their own Frame).
Verified: PCount(1,2,3)=3, PCount(1)=1, PCount()=0
2. Break("string") panicked instead of being caught by RECOVER USING
Root cause: Generated SEQUENCE code only caught *HbError panics.
Break() panics with BreakValue (a different type), which fell
through to EndProc's "runtime error" message and re-panic.
Fix (two parts):
a) gengo emitBeginSequence: recover closure now catches any
panic (interface{}), then dispatches via type switch:
- *HbError → extract .Error() string
- hasValue interface (BreakValue) → extract .GetValue()
- other → static "error" string
b) hbrtl/error.go: BreakValue gets GetValue() method for
duck-type detection without import cycles
c) hbrt/thread.go EndProc: BreakValue type name check added
so it re-panics silently (no stderr noise)
3. SET INDEX TO a, b, c only opened the last file
Root cause: Parser's parseSet() called parseExpr() once for
INDEX setting, stopping at the first comma. Remaining file
names were consumed by the "eat rest of line" loop.
Fix: Parser now collects comma-separated identifiers into a
single string literal "a,b,c". gengo splits on comma and
calls OrderListAdd() for each file.
Verified: SET INDEX TO si_name, si_city → OrdCount=2
All tests pass:
go test ./... 14 packages OK
FiveSql2 43/43 100%
compat_harbour 51/51
Syntax test 100/100
RDD test 44/44
RTL test 114/114
Windows cross-compile OK
Linux cross-compile OK
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Benchmark (50k records, 4 indexes on Apple M-series):
before after Δ
INDEX 53.7ms 33.3ms -38% (now 10% faster than Harbour 37.3ms)
TOTAL 156.2ms 133.0ms -15%
Fixes:
1. sort.Slice(reflection) → concrete sort.Interface
Benchmarked in isolation on 200k KeyRecords:
sort.Slice(closure): 50.0ms
sort.Sort(interface): 30.4ms (40% faster, no reflection)
- indexer.go: add keyRecordAsc/Desc concrete types
- Branch hoist descending check out of Less()
2. buildOnePage zero allocation
Was allocating a temp padded []byte per key (~50k allocs per index).
Now writes padded key directly into the page buffer via padCopy.
3. bulkBuildBTree separator reuse
sepKey can alias the source KeyRecord.Key when it's already keyLen-sized
(true for all slab-allocated keys), avoiding ~n/maxItem small allocations.
Pre-size the children slice.
4. Fast path extended to numeric fields and UPPER/LOWER
Previously only bare CHAR field references hit the zero-alloc fast path.
Now:
- Numeric fields (N/F type) copy DBF bytes directly
(same-length ASCII compare matches numeric order for non-negatives)
- UPPER(field) / LOWER(field) wrappers on CHAR fields apply ASCII
case folding inline during byte copy
Per-index timing on the micro benchmark:
before after
NAME 7.7ms 7.5ms (fast path, unchanged)
CITY 6.0ms 6.2ms (fast path, unchanged)
AGE 14.1ms 7.1ms -50% (was slow path)
UPPER(NM) 17.0ms 7.9ms -54% (was slow path)
5. Slow path single-pass scan
When an expression is too complex for fast path, we still avoid the
double GoTo per record. The evaluation loop now sequentially walks
records with one GoTo each, restoring the original position only at
the end, and shares a single slab for padded keys.
Also fixes a hbrt bug surfaced while writing the benchmark:
6. Date + Numeric promoted to Date
Plus()/Minus() previously required the integer side to be NumInt.
Modulus returns a promoted type, so `SToD("...") + (i % 365)` panicked.
Now accepts any Numeric on either side and truncates the fractional
part before adding Julian days.
- hbrt/ops_arith.go: Date±Numeric (was Date±NumInt only)
Tests:
go test ./... — ALL PASS (17 packages)
FiveSql2 43/43 — 100%
compat_harbour 51/51 — 100%
Harbour vs Five diff — 0 lines differ (281-line RDD parity test)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
From senior Go developer review:
C7 CRITICAL: pagePool data race (ntx.go)
- Moved global pagePool[8] + pagePoolIdx into per-Index struct
- Eliminates race condition across goroutines using separate indexes
C8 CRITICAL: Page.data dangling pointer after remap (ntx.go)
- remapFile() now clears pagePool data slices (pointed into old mmap)
- Prevents segfault from stale mmap references
C4 HIGH: pop() bounds check restored (thread.go)
- Removed performance optimization that eliminated underflow detection
- Stack underflow now produces clear error instead of index -1 panic
C1 HIGH: intExpLen overflow on MinInt64 (value.go)
- Added special case: MinInt64 returns 20 (length of -9223372036854775808)
- Prevents -v overflow in negation
C11 CRITICAL: GoTo ReadAt error handling (dbf.go)
- ReadAt failure now returns error and sets EOF
- Previously silently used stale record buffer (data corruption risk)
C14 HIGH: LEN() inline missing Hash case (gengo.go)
- Added _v.IsHash() → len(Keys) branch
C15 HIGH: EMPTY() inline missing Date case (gengo.go)
- Added _v.IsDate() && _v.AsJulian() == 0 check
82/82 stress PASS. 14 packages ALL PASS.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
value.go:
- cachedNil, cachedTrue, cachedFalse: pre-built constant Values
- MakeBool()/MakeNil(): return cached (zero allocation)
- smallInts[256]: pre-built integers 0-255 (skip intExpLen loop)
- MakeInt(): fast path for 0-255
thread.go:
- pop(): use cachedNil for GC help (no MakeNil() call)
ops_compare.go:
- LessEqual(): inline Int-Int fast path (skip valueCompare)
Direct scalar comparison with cached bool result
- Not(): inline logical fast path (skip IsLogical+AsBool)
- PopLogical(): inline type check + scalar read
Impact: these functions called millions of times in FOR/DO WHILE loops.
10K SEEK: 20ms → 16ms (20%). CDX SCOPE: 12ms → 9ms (25%).
82/82 stress PASS. 14 packages ALL PASS.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CDX Integration:
- IndexEngine interface: common for NTX Index and CDX Tag
- OrderListAdd: auto-detects .cdx/.ntx extension, opens CDX tags
- decodeCompoundLeaf: proper bit-packed tag directory decoding
(was stub falling through to scanCompoundLeaves with wrong names)
- CDX Tag: added KeyLen(), KeyExpr(), ForExpr(), IsDescending(), Close()
- CDX compound recNo = direct byte offset (not page number)
ORDSCOPE:
- SetScope/ClearScope/SetScopeTop/SetScopeBottom on DBFArea
- GoTopIndexed: seeks to scopeTop, validates within scopeBottom
- GoBottomIndexed: seeks to scopeBottom boundary
- SkipIndexed: stops at scope boundaries (top and bottom)
- OrdScope RTL function registered (nScope: 0=TOP, 1=BOTTOM)
- scopeKeyFromValue: converts Value to padded key bytes
Index Order Management:
- OrderListFocus: handles numeric order ("2" → order 2)
- SET ORDER TO n: gengo emits hbrt.NtoS for int-to-string conversion
- IndexOrd/OrdCount/OrdName/OrdKey: real implementations (were stubs)
- OrderCount/CurrentOrder/OrderName/OrderKeyExpr accessors on DBFArea
- ClearScope on order switch (prevents stale scope)
Cross-read test: Harbour-created CDX → Five reads, 20/20 items match:
NAME/CITY/ID seek, ORDSCOPE count, GoTop/GoBottom all identical
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bug 1: FIELD->NAME in INDEX ON expression
- evalKeyExprInner: strip FIELD->/alias-> prefix before field lookup
- exprToString: handle AliasExpr (FIELD->NAME → "FIELD->NAME")
Bug 2: AsNumInt() on Double returned IEEE 754 raw bits
- Value.AsNumInt(): check tDouble and convert via Float64frombits
- Fixed array index crash when index is result of % modulo
Bug 3: PACK/ZAP crash with open indexes
- OrderListRebuild: fully implemented (was TODO stub)
Saves index info, closes all, sets idxState=nil, recreates
- OrderCreate: set current=-1 during key evaluation (natural GoTo)
- PACK/ZAP: save/restore idxState, rebuild after operation
- Register __DBPACK, __DBZAP, DBRECALL symbol aliases
Harbour vs Five: 45/47 match (96%), 2 diffs are duplicate-key sort order
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Complete Harbour-compatible MEMVAR implementation:
- PUBLIC: global scope, persist until program end
- PRIVATE: function scope + called functions, auto-release on return
- Shadowing: PRIVATE can shadow PUBLIC, restored on scope exit
- Nested: multi-level PRIVATE scoping with save/restore stack
- Thread.PushMemvar/PopMemvar: stack-based memvar access
- Thread.DeclarePublic/DeclarePrivate: declaration helpers
- MacroEval: &cVar now looks up memvars (was returning string)
- Shutdown: Phase 4 clears all memvars on all threads
- Case-insensitive: all lookups uppercased
Tests: 12 tests including:
PUBLIC create/update, case-insensitive, PRIVATE basic,
shadow/restore, nested 3-level shadow, new var cleanup,
release, releaseAll, names, thread integration, macro access
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
