FuzzRandomEdits found arrays through each other's cells that settled by the order they were computed in: - Blocked by a cycle, an array shows #REF!, and another reading it can come out smaller for it, clear of the first's inputs, freeing the first, which spills and blocks it again. The pair went around until the passes ran out, stopping wherever that was. An array a cycle blocks twice in a recalculation, freed between, is now held blocked for the rest of it, so they end blocked together, as when the cycle is found in one go. - A formula typed reading an array's cells could close a cycle through another array that isn't computed again for it. An array still blocked by a cycle has every array checked again, once a recalculation. - An array blocked by a cycle counts as spilling over the cells it needs; when its formula gives way to a value, the arrays blocked by a cycle through those cells are now checked again. With the cycles found from the formulas as written and the regions in each other's way, this ships the roadmap's item: fuzzing random edits for 400 seconds finds nothing, and -randedit.seeds=20000 passes.
- commit
- 60442de7cf9ca95a56eee8935e2ebe0a9acedbed
- parent
- d2f45924546bd775a1d19a0f77e4f1574e45d3d0
- author
- Dami <dami.etoile@gmail.com>
- kind
- fix risk score 1.6 of 3, Jev's estimate
@@ -58,7 +58,6 @@ shared editing in section 3.
5858 | Item | Result | Size |5959 |---|---|---|6060 | A workbook of formulas where 012 matches Sheets on purpose (dates, text, rounding, errors, spills), results checked once in Sheets and asserted after; differences chosen on purpose listed in the docs | Formula results people rely on stay put | S |61-| Cycles found from the formulas as written rather than as evaluation walks them: a cycle behind an error that ends a range read or an IF branch not taken, and arrays and notebook outputs in each other's way, settle the same whatever order they're computed in (`-randedit.seeds=20000` still finds about one sequence in 2,500 that reopens differently, and `randKnown` skips one of the default seeds) | Every workbook reads the same when reopened | M |6261 | A week of real use by the owner, problems triaged into this page | Rough edges found by use | S |6362 6463 ### 5. Around the grid@@ -96,6 +95,7 @@ stream above rather than a design of their own.
9695 - An Excel-sized grid in compact column storage, with a ten-million-cell `max-cells` budget; operations cost the data, not the grid: [Bounds of support](docs/contributing/limits.md#sheet-size)9796 - Undo steps in the compact form: clearing a full ten-million-cell sheet holds about what the sheet does, and a step past 1 GB asks first: [Bounds of support](docs/contributing/limits.md#undo)9897 - Spilled cells and pivot results in the compact form, about 20 B each: [Bounds of support](docs/contributing/limits.md#sheet-size)98+- Cycles found from the formulas as written, and arrays and notebook outputs in each other's way settled the same in any order, so a workbook reads the same when reopened: [Formulas](docs/formulas/README.md#values-and-errors)9999 100100 **Finding and using features**101101 @@ -66,7 +66,9 @@ the same cells, blank ones of theirs included, the one whose formula
6666 comes first, row by row, spills and the other shows `#REF!`, whichever6767 was typed first, so a sheet looks the same when opened again. An array that would6868 spill into cells its formula reads, `=SORT(B8:D9)` in A9, is a circular69-dependency and shows `#REF!` too.69+dependency and shows `#REF!` too, as are arrays that would spill into70+each other's inputs, even when one of them, blocked and read as71+`#REF!`, would come out smaller and free the other.7072 7173 ## Names in a formula: LET and LAMBDA7274 @@ -88,6 +88,7 @@ func (w *Workbook) noteSpill(s *Sheet, a Addr, c *Cell, arr *functions.Array) {
8888 // sheets whose pivots to refresh: stale, and those whose source a spill8989 // changed.9090 func (w *Workbook) settleSpills(stale []*Sheet) []*Sheet {91+ defer func() { w.spillCycles, w.spillRechecked = nil, false }()9192 for pass := 0; len(w.spillWork) > 0; pass++ {9293 var changed, anchors []loc9394 for _, l := range w.spillOrder() {@@ -133,24 +134,24 @@ func (w *Workbook) spillOrder() []loc {
133134 func (s *Sheet) applySpill(a Addr, p pendingSpill) ([]loc, bool) {134135 c := s.cells.get(a)135136 if c == nil || !c.IsFormula() || p.arr == nil {136- return s.dropSpill(a), false137+ return s.forgetSpill(a), false137138 }138139 area, why, later := s.spillArea(a, p.arr)139140 var cycle []loc140141 circ := false142+ old := s.spills[a]141143 if why == "" {142144 // Checked each time the anchor is computed: a formula typed since143145 // it spilled may lead from its cells back to it.144- why, cycle = s.spillsIntoItself(a, area)146+ why, cycle = s.spillCycle(a, area, old)145147 circ = why != ""146148 }147- old := s.spills[a]148149 if why == "" && len(later) == 0 && old != nil && old.why == "" && !old.stale && old.auto == c.auto && sameArray(old.arr, p.arr) {149150 return nil, false // written already, and nothing has changed in its way150151 }151152 var changed []loc152- if circ != (old != nil && old.circular) {153- changed = s.wb.recheckArrays(loc{s, a}) // a cycle through arrays began or ended here153+ if s.wb.cycleMoved(circ, old) {154+ changed = s.wb.recheckArrays(loc{s, a})154155 }155156 if why != "" {156157 changed = append(changed, s.dropSpill(a)...)@@ -375,7 +376,7 @@ func (s *Sheet) spillTouched(a Addr, c *Cell) {
375376 return376377 }377378 if s.spills[a] != nil && (c == nil || !c.IsFormula()) {378- for _, l := range s.dropSpill(a) {379+ for _, l := range s.forgetSpill(a) {379380 s.wb.markDirty(l)380381 }381382 }@@ -49,6 +49,67 @@ func (s *Sheet) spillsIntoItself(a Addr, area Rect) (string, []loc) {
4949 return "", nil5050 }5151 52+// spillCycle is spillsIntoItself for the array at a, whose spill was53+// old, holding it blocked once a cycle has blocked it twice in the54+// recalculation, freed between. Arrays whose sizes follow what they read55+// can go around a cycle without settling: blocked, one shows #REF!,56+// which the other reads, and its smaller array frees the first, which57+// spills and blocks it again. Held, they end blocked together, as they58+// are when the cycle is found in one go, whichever came first.59+func (s *Sheet) spillCycle(a Addr, area Rect, old *spill) (string, []loc) {60+ l, w := loc{s, a}, s.wb61+ why, cycle := s.spillsIntoItself(a, area)62+ blocked := old != nil && old.circular63+ checked := w.checkedCycle(l)64+ switch {65+ case why != "" && (!blocked || !checked):66+ w.noteCycle(l)67+ case why == "" && blocked && w.spillCycles[l] >= 2:68+ why = old.why69+ }70+ return why, cycle71+}72+73+// checkedCycle reports whether the array at l was checked for a cycle74+// already in the recalculation, and notes that it has been.75+func (w *Workbook) checkedCycle(l loc) bool {76+ if _, ok := w.spillCycles[l]; ok {77+ return true78+ }79+ if w.spillCycles == nil {80+ w.spillCycles = map[loc]int{}81+ }82+ w.spillCycles[l] = 083+ return false84+}85+86+// cycleMoved reports whether the arrays are to be checked again, the87+// array whose spill was old found blocked by a cycle (circ) or not: a88+// cycle through arrays began or ended at it, which may take in or let go89+// of others, or it is blocked still, the first time in the90+// recalculation: something changed on the way around its cycle, as a91+// formula typed reading another array's cells, which may have closed a92+// cycle through another array, which isn't computed again for it.93+func (w *Workbook) cycleMoved(circ bool, old *spill) bool {94+ switch was := old != nil && old.circular; {95+ case circ != was:96+ return true97+ case circ && !w.spillRechecked:98+ w.spillRechecked = true99+ return true100+ }101+ return false102+}103+104+// noteCycle counts the array at l found blocked by a cycle, from spilling105+// or not, in the recalculation.106+func (w *Workbook) noteCycle(l loc) {107+ if w.spillCycles == nil {108+ w.spillCycles = map[loc]int{}109+ }110+ w.spillCycles[l]++111+}112+52113 // spillWalk is the state of spillsIntoItself: the formulas reached, in53114 // the order found, each with how it was reached.54115 type spillWalk struct {@@ -168,6 +229,19 @@ func (w *Workbook) recheckCircular() []loc {
168229 return out169230 }170231 232+// forgetSpill drops the anchor at a's spill for good, its formula233+// computing one value or none, returning the cells that changed. When a234+// cycle blocked it, it counted as spilling over the cells it needed, so235+// the arrays blocked by a cycle are checked again.236+func (s *Sheet) forgetSpill(a Addr) []loc {237+ old := s.spills[a]238+ changed := s.dropSpill(a)239+ if old != nil && old.circular {240+ changed = append(changed, s.wb.recheckCircular()...)241+ }242+ return changed243+}244+171245 // blockCircular blocks the arrays at l, part of a cycle with another,172246 // and returns the cells that changed, with the anchors, to compute173247 // again, finding the cycle themselves.@@ -181,6 +255,7 @@ func blockCircular(arrays []loc) []loc {
181255 area := sp.area182256 changed = append(changed, l.s.dropSpill(l.a)...)183257 l.s.setSpill(l.a, &spill{area: area, why: "Circular dependency through another array", circular: true})258+ l.s.wb.noteCycle(l)184259 changed = append(changed, l)185260 }186261 return changed@@ -0,0 +1,66 @@
1+package sheet2+3+import "testing"4+5+// Cycles through arrays settle the same typed in any order, recalculated6+// or opened (spillblock.go).7+8+// wantSettled types cells in order, then checks what the cells of want9+// show, as typed, once everything is recalculated, and opened.10+func wantSettled(t *testing.T, name string, order []string, cells, want map[string]string) {11+ t.Helper()12+ s := New()13+ for _, a := range order {14+ s.Set(at(a), cells[a])15+ }16+ check := func(how string, sh *Sheet) {17+ t.Helper()18+ for a, v := range want {19+ if got := sh.Value(at(a)).String(); got != v {20+ t.Errorf("%s, typed %v, %s: %s = %q, want %q", name, order, how, a, got, v)21+ }22+ }23+ }24+ check("typed", s)25+ reopened := roundTrip(t, s)26+ check("reopened", reopened)27+ s.RecalcAll()28+ check("recalculated", s)29+}30+31+// Two arrays reading each other's cells, one's size following what it32+// reads: blocked, each shows #REF!, which the other reads, and its33+// smaller array would free the first. They end blocked together.34+func TestArraysGoingAroundACycle(t *testing.T) {35+ cells := map[string]string{"C3": "=SORT(C13:E16)", "D13": "-5", "B14": "=SORT(C2:E5)"}36+ want := map[string]string{"C3": "#REF!", "B14": "#REF!", "D3": ""}37+ for _, order := range [][]string{{"C3", "D13", "B14"}, {"B14", "D13", "C3"}, {"D13", "C3", "B14"}} {38+ wantSettled(t, "around", order, cells, want)39+ }40+}41+42+// A formula typed last that reads an array's cells closes a cycle43+// through it and another array, which is computed again for it: both44+// are blocked, as when the file opens.45+func TestCycleClosedThroughAnotherArray(t *testing.T) {46+ cells := map[string]string{"D9": "=SORT(G15:H19)", "H19": "=SORT(D13:F16)", "D16": "=ABS($A$9)+1", "G16": `=COUNTIF(H20,">2")`}47+ wantSettled(t, "closed", []string{"D9", "H19", "D16", "G16"}, cells, map[string]string{"D9": "#REF!", "H19": "#REF!"})48+}49+50+// An array blocked by a cycle whose formula gives way to a value frees51+// the arrays the cycle went through.52+func TestCycleGoneWithAnArray(t *testing.T) {53+ s := sheetOf(t, map[string]string{"A9": "=SORT(B9:D12)"})54+ s.Set(at("C9"), "=SORT(A11:C14)")55+ s.Set(at("C11"), `=COUNTIF(A10:A13,">2")`)56+ if v := s.Value(at("C9")); v != ErrRef {57+ t.Fatalf("C9 in a cycle = %v", v)58+ }59+ s.Set(at("A9"), "-5")60+ if sp := s.spills[at("C9")]; sp == nil || sp.why != "" {61+ t.Errorf("C9 once A9 is a value: %+v", sp)62+ }63+ if sp := roundTrip(t, s).spills[at("C9")]; sp == nil || sp.why != "" {64+ t.Errorf("C9 opened: %+v", sp)65+ }66+}@@ -41,6 +41,12 @@ type Workbook struct {
4141 Circular bool4242 // cycles keeps the buffers finding cycles takes (cyclefind.go).4343 cycles *cycleGraph44+ // spillCycles counts, while arrays settle, the times each was found45+ // blocked by a cycle (spillblock.go).46+ spillCycles map[loc]int47+ // spillRechecked is set once an array still blocked by a cycle has48+ // had every array checked again in the recalculation.49+ spillRechecked bool4450 // circArrays counts the arrays blocked by a cycle (spillblock.go),4551 // so arrays that stop spilling look for them only when there are.4652 circArrays int