fix(gengo): compound assign for non-LOCAL LHS
Audit follow-up after Wave 1's pcode `+=` fix surfaced a parallel
class of silent miscompiles in the *gengo* (native-Go) emit path.
Three real bugs hiding behind happy-path test coverage:
* `arr[i] += x` was ASSIGN-only — the IndexExpr branch returned
after emitting `arr[i] := x`, dropping the original element.
Now: PushArray + Push index, ArrayPush to read, fold with RHS,
re-do PushArray + index, ArrayPop to store.
* `alias->field += x` (and the M-> / MEMVAR-> namespace variants)
were ASSIGN-only too. Same shape of bug — `x->v += 7` compiled
as `x->v := 7`. Compound branch reads via PushAliasField (or
PushMemvar for M->), folds, stores via SetAliasField (or
PopMemvar).
* PRIVATE / PUBLIC mid-function declarations were treated as
extra LOCAL slots. emitMidVarDecl extended `locals` past the
function's declared count and emitted `PopLocalFast(idx)` for
the init. The slot didn't exist at runtime, so the init either
silently scribbled past the frame (small N) or panicked with
"local variable index out of range" once exercised. New logic:
PRIVATE/PUBLIC declarations bypass the locals table and emit
`PopMemvar(name)` for the init expression. The runtime auto-
creates the memvar.
* Memvar assignment fallback. After the LOCAL/STATIC checks miss
in emitAssign, the bottom path used to be a one-line WARN that
emitted RHS + `Pop()` — silently discarding the value. PRIVATE
pSum stayed at its initial value forever. Now: ASSIGN goes
through PopMemvar; compound forms read via PushMemvar, fold,
write back via PopMemvar.
Test fixture (tests/std_ch/test_compound_lhs.prg) covers all four
shapes. The std.ch runner picks it up so the regression suite now
stands at 15/15.
Other gates green:
go test ./... : PASS
FiveSql2 SQL:1999 : 43/43
Harbour compat : 56/56
std.ch suite : 15/15
FRB suite : 5/5
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -323,6 +323,25 @@ func (g *Generator) emitStmt(stmt ast.Stmt, locals localMap) {
|
||||
}
|
||||
|
||||
func (g *Generator) emitMidVarDecl(s *ast.VarDecl, locals localMap) {
|
||||
// PRIVATE / PUBLIC live in the runtime memvar namespace, NOT in
|
||||
// the function's local slot table. Without this distinction the
|
||||
// declaration was silently registered as a phantom LOCAL slot
|
||||
// past the function's declared LOCAL count: subsequent reads/
|
||||
// writes via LocalAdd / PopLocalFast then panicked with "local
|
||||
// variable index out of range" (or, worse, silently scribbled
|
||||
// past the allocated frame).
|
||||
if s.Scope == ast.ScopePrivate || s.Scope == ast.ScopePublic {
|
||||
for _, v := range s.Vars {
|
||||
if v.Init == nil {
|
||||
// Bare PRIVATE/PUBLIC declaration without init —
|
||||
// runtime auto-creates the memvar on first read/write.
|
||||
continue
|
||||
}
|
||||
g.emitExpr(v.Init)
|
||||
g.writeln(fmt.Sprintf(`t.PopMemvar(%q)`, v.Name))
|
||||
}
|
||||
return
|
||||
}
|
||||
for _, v := range s.Vars {
|
||||
idx, found := locals[strings.ToUpper(v.Name)]
|
||||
if !found {
|
||||
@@ -423,7 +442,7 @@ func (g *Generator) emitExprStmt(s *ast.ExprStmt, locals localMap) {
|
||||
}
|
||||
|
||||
func (g *Generator) emitAssign(a *ast.AssignExpr, locals localMap) {
|
||||
// Check for arr[idx] := value (array index assignment)
|
||||
// Check for arr[idx] := / += / -= / etc. (array index assignment)
|
||||
if idx, ok := a.Left.(*ast.IndexExpr); ok {
|
||||
if a.Op == token.ASSIGN {
|
||||
g.emitExpr(idx.X) // array
|
||||
@@ -432,6 +451,22 @@ func (g *Generator) emitAssign(a *ast.AssignExpr, locals localMap) {
|
||||
g.writeln("t.ArrayPop()") // set array[index] = value
|
||||
return
|
||||
}
|
||||
// Compound: read current arr[idx], fold with RHS, store back.
|
||||
// Without this, `arr[i] += x` was silently compiled as
|
||||
// `arr[i] := x` — the original element discarded. Same fix
|
||||
// pattern as the LOCAL / STATIC compound branches below.
|
||||
g.emitExpr(idx.X)
|
||||
g.emitExpr(idx.Index)
|
||||
g.writeln("t.ArrayPush()") // push arr[index] for read
|
||||
g.emitExpr(a.Right)
|
||||
g.emitBinaryOp(a.Op)
|
||||
// Stack now: [folded value]. Re-push X/index to set.
|
||||
g.writeln("_v := t.Pop2()")
|
||||
g.emitExpr(idx.X)
|
||||
g.emitExpr(idx.Index)
|
||||
g.writeln("t.PushValue(_v)")
|
||||
g.writeln("t.ArrayPop()")
|
||||
return
|
||||
}
|
||||
|
||||
// Check for obj:field := value (object field assignment)
|
||||
@@ -481,7 +516,7 @@ func (g *Generator) emitAssign(a *ast.AssignExpr, locals localMap) {
|
||||
}
|
||||
}
|
||||
|
||||
// Check for alias->field := value (FIELD->NAME := value)
|
||||
// Check for alias->field := / += / etc. (workarea field assign)
|
||||
if aliasExpr, ok := a.Left.(*ast.AliasExpr); ok {
|
||||
if aliasIdent, ok2 := aliasExpr.Alias.(*ast.IdentExpr); ok2 {
|
||||
if fieldIdent, ok3 := aliasExpr.Field.(*ast.IdentExpr); ok3 {
|
||||
@@ -489,11 +524,30 @@ func (g *Generator) emitAssign(a *ast.AssignExpr, locals localMap) {
|
||||
// `M->name := v` / `MEMVAR->name := v` are memvar writes,
|
||||
// not workarea field writes.
|
||||
if upper == "M" || upper == "MEMVAR" {
|
||||
if a.Op == token.ASSIGN {
|
||||
g.emitExpr(a.Right)
|
||||
g.writeln(fmt.Sprintf(`t.PopMemvar(%q)`, fieldIdent.Name))
|
||||
return
|
||||
}
|
||||
// Compound: M-> is the memvar namespace.
|
||||
g.writeln(fmt.Sprintf(`t.PushMemvar(%q)`, fieldIdent.Name))
|
||||
g.emitExpr(a.Right)
|
||||
g.emitBinaryOp(a.Op)
|
||||
g.writeln(fmt.Sprintf(`t.PopMemvar(%q)`, fieldIdent.Name))
|
||||
return
|
||||
}
|
||||
if a.Op == token.ASSIGN {
|
||||
g.emitExpr(a.Right)
|
||||
g.writeln(fmt.Sprintf(`{ _wa := t.WA.(*hbrdd.WorkAreaManager); _wa.SetAliasField(%q, %q, t.Pop2()) }`, aliasIdent.Name, fieldIdent.Name))
|
||||
return
|
||||
}
|
||||
// Compound: read current alias->field, fold with RHS,
|
||||
// write back. `x->v += 7` used to compile as `x->v := 7`
|
||||
// (silent miscompile) — the original field value got
|
||||
// discarded.
|
||||
g.writeln(fmt.Sprintf(`t.PushAliasField(%q, %q)`, aliasIdent.Name, fieldIdent.Name))
|
||||
g.emitExpr(a.Right)
|
||||
g.emitBinaryOp(a.Op)
|
||||
g.writeln(fmt.Sprintf(`{ _wa := t.WA.(*hbrdd.WorkAreaManager); _wa.SetAliasField(%q, %q, t.Pop2()) }`, aliasIdent.Name, fieldIdent.Name))
|
||||
return
|
||||
}
|
||||
@@ -573,6 +627,21 @@ func (g *Generator) emitAssign(a *ast.AssignExpr, locals localMap) {
|
||||
}
|
||||
return
|
||||
}
|
||||
// Memvar fallback (PRIVATE / PUBLIC / unresolved IDENT) —
|
||||
// runtime auto-creates the memvar on assign. Without this
|
||||
// the bottom WARN fallback dropped the RHS and silently
|
||||
// did nothing, so `PRIVATE pSum := 50 ; pSum += 25` left
|
||||
// pSum at 50.
|
||||
if a.Op == token.ASSIGN {
|
||||
g.emitExpr(a.Right)
|
||||
g.writeln(fmt.Sprintf(`t.PopMemvar(%q)`, ident.Name))
|
||||
return
|
||||
}
|
||||
g.writeln(fmt.Sprintf(`t.PushMemvar(%q)`, ident.Name))
|
||||
g.emitExpr(a.Right)
|
||||
g.emitBinaryOp(a.Op)
|
||||
g.writeln(fmt.Sprintf(`t.PopMemvar(%q)`, ident.Name))
|
||||
return
|
||||
}
|
||||
// Fallback: general assignment via stack
|
||||
g.emitExpr(a.Right)
|
||||
|
||||
@@ -30,6 +30,7 @@ TESTS=(
|
||||
test_set_deleted
|
||||
test_unsupported
|
||||
test_block_comma
|
||||
test_compound_lhs
|
||||
)
|
||||
|
||||
work="$(mktemp -d)"
|
||||
|
||||
36
tests/std_ch/test_compound_lhs.prg
Normal file
36
tests/std_ch/test_compound_lhs.prg
Normal file
@@ -0,0 +1,36 @@
|
||||
PROCEDURE Main()
|
||||
LOCAL aArr := { 10, 20, 30 }, oObj, n
|
||||
|
||||
/* 1. array index += */
|
||||
aArr[2] += 5
|
||||
? "1. aArr[2] += 5 =>", aArr[2], "(expect 25)"
|
||||
|
||||
/* 2. alias->field += (use a real workarea) */
|
||||
FErase("c.dbf")
|
||||
dbCreate("c.dbf", { {"V", "N", 6, 0} })
|
||||
USE c.dbf NEW EXCLUSIVE ALIAS x
|
||||
dbAppend(); FieldPut(1, 100)
|
||||
dbCommit()
|
||||
|
||||
x->v += 7
|
||||
? "2. x->v += 7 =>", x->v, "(expect 107)"
|
||||
|
||||
x->(dbCloseArea())
|
||||
FErase("c.dbf")
|
||||
|
||||
/* 3. memvar / private += */
|
||||
PRIVATE pSum := 50
|
||||
pSum += 25
|
||||
? "3. PRIVATE pSum += 25 =>", pSum, "(expect 75)"
|
||||
|
||||
/* 4. STATIC += (already worked per gengo source, sanity) */
|
||||
StaticTest()
|
||||
|
||||
? "DONE"
|
||||
RETURN
|
||||
|
||||
STATIC FUNCTION StaticTest()
|
||||
STATIC s_n := 100
|
||||
s_n += 5
|
||||
? "4. STATIC s_n += 5 =>", s_n, "(expect 105)"
|
||||
RETURN NIL
|
||||
Reference in New Issue
Block a user