Reduce of zero returns a removed-zeros count taken from the destination, not the operand

perfloop-oss/apd · DERIVED SUMMARY DRIFT

https://perfloop.ai/t/oss/case_2z5yd6abh2

Verdict

VERIFIED · settled 2026-10-29 · pull request opened as cockroachdb/apd#167

What happened: The assertion is violated on the comparison and satisfied with this change.

Hypothesis

Reduce of zero may return a removed-zeros count taken from the destination's previous digits rather than from the operand. Callers that reuse a destination Decimal may receive a wrong count. The submitter describes reuse as a common apd pattern to avoid allocation; its prevalence and downstream impact remain unmeasured.

The submitter reports reproducing this on upstream master e9c7a71: `d := New(10,0); _, n := d.Reduce(New(0,0))` returns `n=1`, while `new(Decimal).Reduce(New(0,0))` returns `n=0`. The Case must independently check these reported outputs.

Reduce documents that it returns the number of zeros removed from x (decimal.go:708–710). In `(*Decimal).Reduce`, the `Sign()==0` branch computes `nd = int(d.NumDigits())` from the destination before `d.SetInt64(0)` overwrites it, then returns `nd-1` (decimal.go:717–721). NumDigits counts coefficient digits and returns 1 for zero (table.go:66–78). The current TestReduce cases call `d.Reduce(d)` (decimal_test.go:949–984), so they do not cover a distinct reused destination.

The smallest useful check is a regression in TestReduce, run with `go test -run '^TestReduce$' -count=1 .` on unchanged master. Compare a destination initialized to `New(10,0)` with a fresh Decimal, using `New(0,0)`, parsed `0E+3`, and parsed `000`; require `n=0` and the reduced zero value. A failure only for the reused destination would confirm the reported dependence; identical correct counts would reject that reproduction. Then run the same regression and existing Reduce cases after the minimal repair.

Change to test: In the Sign()==0 case use x.NumDigits() instead of d.NumDigits(), so the count is derived from the operand. Add a regression test that reuses a non-zero destination, covers zero with a non-zero exponent such as 0E+3 as well as 000, and fails on master; keep the change minimal.

Where it lives

perfloop-oss/apd

Evidence

The assertion is violated on the comparison and satisfied with this change: `For finite zero operands New(0,0), parsed 0E+3, and parsed 000, Decimal.Reduce reports 0 removed zeros and writes canonical zero to a distinct destination, whether it starts fresh or as New(10,0).`

Checks: 1 of 1 passed. Verification: no defect found.

Timeline