fivedev/five
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: PCount, Break/RECOVER, SET INDEX TO — 3 Harbour compat fixes

Browse Source 
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>
This commit is contained in:
CharlesKWON
2026-04-13 18:06:28 +09:00
parent ad544a5528
commit 3adc9d7d59
5 changed files with 102 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
compiler/gengo/gengo.go
View File

@@ -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 {

29
compiler/parser/parser.go
View File

@@ -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()

33
hbrt/thread.go
View File

@@ -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

6
hbrtl/error.go
View File

@@ -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()

5
hbrtl/missing.go
View File

@@ -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.