Skip to content

Commit

Permalink
Fix handling of inherited cells for empty prefix
Browse files Browse the repository at this point in the history
Before this change, when a wad was moved from the prefix to the
suffix, its list of inherited cells was unconditionally cleared. As a
result, when the prefix consisted of only one wad and that wad was
moved from the prefix to the suffix and then back from the suffix to
the prefix, the wad ended up without a list of inherited cells while
its list of inherited cells should have been the initial reader state
instead. Another failure mode was that removing the last remaining
wad from the prefix (without moving to the suffix) could lead to the
first suffix wad having invalid inherited cells.

After this change, when a wad is moved from the prefix to the suffix,
its list of inherited cells is cleared only if there is still at least
one wad on the prefix. Similarly, if the last remaining wad is removed
from the prefix, the inherited cells of the first wad of the suffix
are set to the initial state of the analyzer.
  • Loading branch information
scymtym committed Dec 6, 2024
1 parent d95bb89 commit b663ce9
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 22 deletions.
24 changes: 14 additions & 10 deletions code/cache.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,17 @@
(old-prefix-width (prefix-width cache))
(old-width (first old-prefix-width))
(new-width (max-line-width wad)))
(if (null old-prefix-top)
(alexandria:maxf
new-width (max-line-length cache 0 (1- (start-line wad))))
(if (not (null old-prefix-top))
(setf (dep:inherited wad) (dep:inheritable old-prefix-top))
(assert (not (eq :invalid (dep:inherited wad)))))
(if (not (null old-prefix-top))
(alexandria:maxf
new-width
old-width
(max-line-length
cache (1+ (end-line old-prefix-top)) (1- (start-line wad)))))
cache (1+ (end-line old-prefix-top)) (1- (start-line wad))))
(alexandria:maxf
new-width (max-line-length cache 0 (1- (start-line wad)))))
(setf (prefix-width cache) (list* new-width old-prefix-width)
(prefix cache) new-prefix)
(link-siblings old-prefix-top wad)
Expand All @@ -213,18 +216,19 @@
;; 1) the absolute line numbers in WAD itself and its
;; descendants are potentially invalid.
;; 2) the list of inherited cells in WAD is not populated
;; We handle 1) here while 2) is handled by `push-to-prefix'.
(compute-absolute-line-numbers wad)
(setf (dep:inherited wad) (dep:inheritable (first (prefix cache))))
(push-to-prefix cache wad))))

(defgeneric prefix-to-suffix (cache)
(:method ((cache cache))
(let ((wad (pop-from-prefix cache)))
;; During the time WAD will spend on the suffix, its list of
;; inherited cells would not be kept up-to-date. So we clear
;; the slot here and recompute a new value when WAD is moved to
;; the prefix.
(setf (dep:inherited wad) :invalid)
;; During the time WAD will spend on the suffix, unless the
;; prefix is empty, its list of inherited cells would not be
;; kept up-to-date. So we clear the slot here and recompute a
;; new value when WAD is moved to the prefix.
(unless (null (prefix cache))
(setf (dep:inherited wad) :invalid))
(push-to-suffix cache wad))))

(defun pop-from-worklist (cache)
Expand Down
30 changes: 18 additions & 12 deletions code/read.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,14 @@
;; available and thus the users must be invalidated.
(dep:update-escaping-for-toplevel-user
wad #'invalidate-following-users)
;; Set inherited cells of WAD either to the inheritable
;; cells of the preceding wad or the initial reader
;; state. The inherited cells are used when parsing
;; starts after WAD and a reader state for that location
;; has to be computed.
(setf (dep:inherited wad)
(alexandria:if-let ((prefix (prefix cache)))
(dep:inheritable (first prefix))
(initial-reader-state analyzer)))
;; Add the fully processed WAD to the prefix of CACHE.
;; Add the WAD to the prefix of CACHE. `push-to-prefix'
;; sets the inherited cells of WAD either to the
;; inheritable cells of the preceding wad or the initial
;; reader state. The inherited cells are used when
;; parsing starts after WAD and a reader state for that
;; location has to be computed.
(when (null (prefix cache))
(setf (dep:inherited wad) (initial-reader-state analyzer)))
(push-to-prefix cache wad)))))
(values kind wad)))

Expand Down Expand Up @@ -207,11 +205,19 @@
:do (alexandria:when-let ((residue (residue cache)))
(mapc #'detach-wad residue)
(setf (residue cache) '()))
(return-from read-forms nil)
(return)
;; In case we skipped some whitespace, discard any wads on
;; the cache residue and cache suffix that are now before
;; the current stream position.
:unless (eq kind :whitespace)
:do (drain-result-list analyzer cache residue pop-from-residue)
(when (null (residue cache))
(drain-result-list analyzer cache suffix pop-from-suffix))))))
(drain-result-list analyzer cache suffix pop-from-suffix))))
;; If the buffer changed such that the prefix was previously not
;; empty and is now empty and the suffix is non-empty, ensure that
;; the first element of the suffix has the initial reader state as
;; its set of inherited cells.
(when (null (prefix cache))
(alexandria:when-let ((suffix-top (first (suffix cache))))
(when (eq (dep:inherited suffix-top) :invalid)
(setf (dep:inherited suffix-top) (initial-reader-state analyzer)))))))
17 changes: 17 additions & 0 deletions test/regressions.lisp
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,20 @@ another wad."
'((inc:atom-wad ((0 0) (1 2))
(:errors ((((1 2) (1 2)) eclector.reader:unterminated-list)))
#1#)))))

(test regressions.delete-first-line
"Deleting the contents of the first buffer line could lead to invalid
inherited cells in the first element of the cache suffix."
(edits-cases ()
(;; A "second" line.
"2
"
:ignore
;; Add a first line before the second line.
'(:insert (0 0) "1
")
:ignore
;; Delete the contents of the first line. This removes the single
;; element of the prefix.
'(:erase (0 0) 1)
:ignore)))

0 comments on commit b663ce9

Please sign in to comment.