Unmerge and Merge had the arrays over the range asked about spill again, but the merges they take away can reach past it, so an array blocked by such a merge stayed blocked until something else touched it. They now ask over the range and every merge taken away. An array found blocked by a cycle through the cells of another array that stops spilling in the same pass (deleting rows puts a plain formula where an array's anchor was, and the anchor's old spill stays until it's computed) stayed blocked, since nothing it reads changed. An array that stops spilling or spills elsewhere now has the arrays blocked by a cycle computed again; a count keeps this free when there are none. Found by TestRandomEdits with -randedit.seeds=20000 (seeds 9887, 10602, 13460, 14621).
- commit
- cbb411ffd26487c4c879c9063c2248f6831ba4c7
- parent
- 85fd593027b2c8132193c416ff4403ea5ae894b4
- author
- Dami <dami.etoile@gmail.com>
- kind
- fix risk score 1.3 of 3, Jev's estimate
| M | internal/ | 28 | +++++++++++++++++----------- |
| M | internal/ | 17 | +++++++++++++++++ |
| M | internal/ | 11 | ++++++++++- |
| M | internal/ | 15 | +++++++++++++++ |
| M | internal/ | 21 | +++++++++++++++++++++ |
| M | internal/ | 3 | +++ |
@@ -183,11 +183,8 @@ func (s *Sheet) Merge(r Rect, k MergeKind) error {
183183 s.place(a, c.leftover())184184 }185185 }186- next := slices.DeleteFunc(slices.Clone(s.view.merges), func(m Rect) bool {187- _, overlaps := intersectRect(m, r)188- return overlaps189- })190- s.setMerges(append(next, areas...), r)186+ next, span := s.mergesOutside(r)187+ s.setMerges(append(next, areas...), span)191188 })192189 return nil193190 }@@ -211,18 +208,27 @@ func (s *Sheet) MergeAcross(rows bool, n int) (Rect, bool) {
211208 // Unmerge splits the merges overlapping r back into cells, as one undo212209 // step, and returns how many there were.213210 func (s *Sheet) Unmerge(r Rect) int {214- n := 0211+ next, span := s.mergesOutside(r)212+ n := len(s.view.merges) - len(next)213+ if n > 0 {214+ s.change("unmerge "+r.String(), r, func() { s.setMerges(next, span) })215+ }216+ return n217+}218+219+// mergesOutside returns the merges that don't overlap r, and the range220+// holding r and those that do: where arrays blocked by them may spill221+// once they go.222+func (s *Sheet) mergesOutside(r Rect) ([]Rect, Rect) {223+ span := r215224 next := slices.DeleteFunc(slices.Clone(s.view.merges), func(m Rect) bool {216225 _, overlaps := intersectRect(m, r)217226 if overlaps {218- n++227+ span = union(span, m)219228 }220229 return overlaps221230 })222- if n > 0 {223- s.change("unmerge "+r.String(), r, func() { s.setMerges(next, r) })224- }225- return n231+ return next, span226232 }227233 228234 // setMerges replaces the merges, recording them for undo, and has the@@ -52,3 +52,20 @@ func TestDeletedMergeFreesArray(t *testing.T) {
5252 t.Errorf("B18 once the merge went = %v", v)5353 }5454 }55+56+// Unmerging frees the arrays blocked by the merges it takes away, where57+// they reach past the range unmerged.58+func TestUnmergeFreesArrayPastTheRange(t *testing.T) {59+ s := New()60+ s.Merge(NewRect(at("H11"), at("H13")), MergeVertically)61+ s.Set(at("H11"), "=SEQUENCE(2)")62+ if v := s.Value(at("H11")); v != ErrRef {63+ t.Fatalf("H11 over a merge = %v", v)64+ }65+ if n := s.Unmerge(NewRect(at("G13"), at("H15"))); n != 1 {66+ t.Fatalf("unmerged %d", n)67+ }68+ if v := s.Value(at("H12")); v != num(2) {69+ t.Errorf("H12 once the merge went = %v", v)70+ }71+}@@ -166,6 +166,9 @@ func (s *Sheet) applySpill(a Addr, p pendingSpill) ([]loc, bool) {
166166 changed = append(changed, s.regionsGiveWay(area)...)167167 if old != nil && old.why == "" {168168 changed = append(changed, s.clearSpilled(old.area, area)...)169+ if old.area != area {170+ changed = append(changed, s.wb.recheckCircular()...)171+ }169172 }170173 s.setSpill(a, &spill{area: area, arr: p.arr, auto: c.auto})171174 c.Value = p.top@@ -338,7 +341,7 @@ func (s *Sheet) dropSpill(a Addr) []loc {
338341 if old.why != "" {339342 return nil340343 }341- return s.clearSpilled(old.area, Rect{From: a, To: a})344+ return append(s.clearSpilled(old.area, Rect{From: a, To: a}), s.wb.recheckCircular()...)342345 }343346 344347 // setSpill records the anchor at a's spill (nil for none), indexing@@ -347,10 +350,16 @@ func (s *Sheet) setSpill(a Addr, sp *spill) {
347350 if old := s.spills[a]; old != nil {348351 s.spillAt.remove(a, []Rect{old.area})349352 delete(s.spills, a)353+ if old.circular {354+ s.wb.circArrays--355+ }350356 }351357 if sp == nil {352358 return353359 }360+ if sp.circular {361+ s.wb.circArrays++362+ }354363 if s.spills == nil {355364 s.spills = map[Addr]*spill{}356365 }@@ -302,3 +302,18 @@ func TestSpillIntoItsInput(t *testing.T) {
302302 }303303 }304304 }305+306+// An array isn't left blocked by a cycle through the cells of an array307+// that stopped spilling in the same recalculation: deleting a row moves308+// the array at A3 to A2, and puts at A3 a formula reading B2 whose309+// array spilled over C3, which G1 reads, until it's computed again.310+func TestNoCycleThroughAnArrayGone(t *testing.T) {311+ s := sheetOf(t, map[string]string{"A3": "=D11:F11", "A4": "=B3", "G2": "=C4", "F11": "=G2"})312+ if sp := s.spills[at("A3")]; sp == nil || sp.why != "" {313+ t.Fatalf("A3's array: %+v", sp)314+ }315+ s.DeleteRows(0, 1)316+ if sp := s.spills[at("A2")]; sp == nil || sp.why != "" {317+ t.Fatalf("A2's array after deleting a row: %+v", sp)318+ }319+}@@ -137,6 +137,27 @@ func (w *Workbook) recheckArrays(l loc) []loc {
137137 return out138138 }139139 140+// recheckCircular returns the anchors of the arrays blocked by a cycle,141+// to compute again, when an array stops spilling or spills over other142+// cells: the cycle may have gone through its cells, and the arrays143+// found it blocked in the same pass, before it changed, which nothing144+// they read may tell them.145+func (w *Workbook) recheckCircular() []loc {146+ if w.circArrays <= 0 {147+ return nil148+ }149+ var out []loc150+ for _, s := range w.sheets {151+ for b, sp := range s.spills {152+ if sp.circular {153+ sp.stale = true154+ out = append(out, loc{s, b})155+ }156+ }157+ }158+ return out159+}160+140161 // blockCircular blocks the arrays at l, part of a cycle with another,141162 // and returns the cells that changed, with the anchors, to compute142163 // again, finding the cycle themselves.@@ -39,6 +39,9 @@ type Workbook struct {
3939 4040 // Circular is set when the last recalculation found a cycle.4141 Circular bool42+ // circArrays counts the arrays blocked by a cycle (spillblock.go),43+ // so arrays that stop spilling look for them only when there are.44+ circArrays int4245 4346 remote RemoteSource // answers JEV functions, see remote.go4447 trace any // the owner's telemetry trace, see observe.go