From 3adc9d7d59e786f65f267cca4d8f255587e4d264 Mon Sep 17 00:00:00 2001 From: CharlesKWON Date: Mon, 13 Apr 2026 18:06:28 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20PCount,=20Break/RECOVER,=20SET=20INDEX?= =?UTF-8?q?=20TO=20=E2=80=94=203=20Harbour=20compat=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- compiler/gengo/gengo.go | 45 +++++++++++++++++++++++++++++++-------- compiler/parser/parser.go | 29 +++++++++++++++++++++++++ hbrt/thread.go | 33 +++++++++++++++++++++++----- hbrtl/error.go | 6 ++++++ hbrtl/missing.go | 5 +++-- 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/compiler/gengo/gengo.go b/compiler/gengo/gengo.go index c4672ee..1957ff1 100644 --- a/compiler/gengo/gengo.go +++ b/compiler/gengo/gengo.go @@ -655,12 +655,20 @@ func (g *Generator) emitStmt(stmt ast.Stmt, locals localMap) { g.writeln("if idx, ok := area.(hbrdd.Indexer); ok {") g.indent++ if fileStr != "" { - // Strip surrounding quotes from string literals + // SET INDEX TO a, b, c — split comma-separated file names + // and call OrderListAdd for each. Harbour loads all NTX + // files into the active index list. clean := fileStr if len(clean) >= 2 && clean[0] == '"' && clean[len(clean)-1] == '"' { clean = clean[1 : len(clean)-1] } - g.writeln(fmt.Sprintf(`idx.OrderListAdd(%q)`, clean)) + parts := strings.Split(clean, ",") + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + g.writeln(fmt.Sprintf(`idx.OrderListAdd(%q)`, p)) + } + } } else { g.emitExpr(s.Expr) g.writeln(`idx.OrderListAdd(t.Pop2().AsString())`) @@ -1329,19 +1337,18 @@ func (g *Generator) emitSwitch(s *ast.SwitchStmt, locals localMap) { func (g *Generator) emitBeginSequence(s *ast.SeqStmt, locals localMap) { // BEGIN SEQUENCE → Go's panic/recover. - // Use a _seqBreak flag to signal Break() was called. - // Break() panics with *HbError, caught by our recover. + // Catches both *HbError (runtime errors) and BreakValue (Break() calls). + // BreakValue is defined in hbrtl, but we detect it via duck typing + // to avoid import cycles. g.writeln("{ // BEGIN SEQUENCE") g.indent++ - g.writeln("_seqErr := func() (_recoverErr *hbrt.HbError) {") + g.writeln("_seqErr := func() (_recoverVal interface{}) {") g.indent++ g.writeln("defer func() {") g.indent++ g.writeln("if r := recover(); r != nil {") g.indent++ - g.writeln("if hbErr, ok := r.(*hbrt.HbError); ok {") - g.writeln(" _recoverErr = hbErr") - g.writeln("} else { panic(r) }") + g.writeln("_recoverVal = r") g.indent-- g.writeln("}") g.indent-- @@ -1362,7 +1369,27 @@ func (g *Generator) emitBeginSequence(s *ast.SeqStmt, locals localMap) { g.indent++ if s.RecoverVar != "" { if idx, found := locals[strings.ToUpper(s.RecoverVar)]; found { - g.writeln(fmt.Sprintf("t.SetLocalFast(%d, hbrt.MakeString(_seqErr.Error()))", idx)) + // Extract the value from the recovered panic: + // *HbError → error description string + // BreakValue (has .Value field) → the Break() argument + // other → string representation + g.writeln(fmt.Sprintf(`{ // RECOVER USING %s`, s.RecoverVar)) + g.indent++ + g.writeln(`switch _sv := _seqErr.(type) {`) + g.writeln(`case *hbrt.HbError:`) + g.writeln(fmt.Sprintf(` t.SetLocalFast(%d, hbrt.MakeString(_sv.Error()))`, idx)) + g.writeln(`default:`) + // For BreakValue, use reflection-free approach: check if + // the type has a Value field via a local interface. + g.writeln(` type hasValue interface{ GetValue() hbrt.Value }`) + g.writeln(` if bv, ok := _sv.(hasValue); ok {`) + g.writeln(fmt.Sprintf(` t.SetLocalFast(%d, bv.GetValue())`, idx)) + g.writeln(` } else {`) + g.writeln(fmt.Sprintf(` t.SetLocalFast(%d, hbrt.MakeString("error"))`, idx)) + g.writeln(` }`) + g.writeln(`}`) + g.indent-- + g.writeln(`}`) } } for _, stmt := range s.RecoverBody { diff --git a/compiler/parser/parser.go b/compiler/parser/parser.go index d3e6cfe..c000de1 100644 --- a/compiler/parser/parser.go +++ b/compiler/parser/parser.go @@ -1781,6 +1781,35 @@ func (p *Parser) parseSet() *ast.SetCmd { if upperSetting == "FILTER" || upperSetting == "RELATION" || upperSetting == "ORDER" || upperSetting == "INDEX" { if p.current.Kind != token.NEWLINE && p.current.Kind != token.EOF { expr = p.parseExpr() + // SET INDEX TO a, b, c — collect comma-separated file names + // into a single string literal "a,b,c" for gengo to split. + if upperSetting == "INDEX" { + getName := func(e ast.Expr) string { + switch v := e.(type) { + case *ast.IdentExpr: + return v.Name + case *ast.LiteralExpr: + return v.Value + default: + return "" + } + } + combined := getName(expr) + for p.current.Kind == token.COMMA { + p.advance() + if p.current.Kind != token.NEWLINE && p.current.Kind != token.EOF { + next := p.parseExpr() + combined += "," + getName(next) + } + } + if strings.Contains(combined, ",") { + expr = &ast.LiteralExpr{ + Value: combined, + Kind: token.STRING, + ValuePos: expr.Pos(), + } + } + } } if p.current.Kind == token.INTO { p.advance() diff --git a/hbrt/thread.go b/hbrt/thread.go index 429364d..2c9c68d 100644 --- a/hbrt/thread.go +++ b/hbrt/thread.go @@ -225,7 +225,7 @@ func (t *Thread) Frame(params, locals int) { frame.base = t.sp - actual // only actual args on stack frame.localBase = localBase frame.localCount = params + locals - frame.paramCount = params + frame.paramCount = t.pendingParams // actual args passed by caller (not declared count) frame.retVal = MakeNil() frame.symbol = t.pendingCallSym t.pendingCallSym = nil @@ -249,14 +249,23 @@ func (t *Thread) Frame(params, locals int) { // EndProc is called via defer at the end of every function. // Handles recover for BEGIN SEQUENCE and restores frame. -// HbError panics are re-panicked so the generated SEQUENCE handler can catch them. +// All panics are re-panicked so the generated SEQUENCE/RECOVER handler +// can catch them. HbError + BreakValue (from Break() in hbrtl) are +// re-panicked silently; unknown panics also re-panic but with a +// diagnostic message on stderr. func (t *Thread) EndProc() { if r := recover(); r != nil { t.endFrame() if _, ok := r.(*HbError); ok { - panic(r) // re-panic: let BEGIN SEQUENCE's generated recover catch it + panic(r) // HbError — re-panic silently } - fmt.Fprintf(os.Stderr, "Five runtime error: %v\n", r) + // Check for BreakValue from hbrtl.Break() via duck typing. + // We can't import hbrtl (cycle), so we check the type name. + rType := fmt.Sprintf("%T", r) + if rType == "hbrtl.BreakValue" { + panic(r) // BreakValue — re-panic silently for RECOVER USING + } + fmt.Fprintf(os.Stderr, "Five runtime error: %v [recovered, repanicked]\n", r) panic(r) } t.endFrame() @@ -559,11 +568,25 @@ func (t *Thread) VM() *VM { } // ParamCount returns the number of parameters passed to the current call. -// Used by variadic RTL functions (QOut, etc.). +// Used by RTL functions that call ParamCount() BEFORE Frame() — returns +// pendingParams set by Function(nArgs). This is the original behavior +// that all existing RTL functions depend on. +// +// For PRG-level PCount(), use CallerParamCount() instead (via PCount RTL). func (t *Thread) ParamCount() int { return t.pendingParams } +// CallerParamCount returns the param count of the calling PRG function +// (one frame below the current). Used by PCount() RTL which needs the +// caller's count, not its own. +func (t *Thread) CallerParamCount() int { + if t.callSP >= 2 { + return t.calls[t.callSP-2].paramCount + } + return 0 +} + // PendingParams2 sets pending param count for direct block calls (AEval, ASort etc.) func (t *Thread) PendingParams2(n int) { t.pendingParams = n diff --git a/hbrtl/error.go b/hbrtl/error.go index 1737f3e..c507bb0 100644 --- a/hbrtl/error.go +++ b/hbrtl/error.go @@ -103,6 +103,12 @@ type BreakValue struct { Value hbrt.Value } +// GetValue returns the Break() argument. Used by RECOVER USING via duck typing +// (the generated code checks for a `hasValue` interface to avoid import cycles). +func (bv BreakValue) GetValue() hbrt.Value { + return bv.Value +} + // Break(xValue) → panics with BreakValue, caught by BEGIN SEQUENCE/RECOVER. func Break(t *hbrt.Thread) { nParams := t.ParamCount() diff --git a/hbrtl/missing.go b/hbrtl/missing.go index c62e6d3..016d4f5 100644 --- a/hbrtl/missing.go +++ b/hbrtl/missing.go @@ -269,11 +269,12 @@ func TypeFunc(t *hbrt.Thread) { t.RetValue() } -// PCount returns number of parameters passed. +// PCount returns number of parameters passed to the calling PRG function. +// Harbour: hb_pcount() — returns the CALLER's param count, not PCount's own. func PCount(t *hbrt.Thread) { t.Frame(0, 0) defer t.EndProcFast() - t.RetInt(int64(t.ParamCount())) + t.RetInt(int64(t.CallerParamCount())) } // Break moved to error.go — full implementation with BreakValue type.