From b663ce97f172d25a028c08b464d9c0d309a3efaa Mon Sep 17 00:00:00 2001 From: Jan Moringen Date: Fri, 6 Dec 2024 21:14:26 +0100 Subject: [PATCH] Fix handling of inherited cells for empty prefix 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. --- code/cache.lisp | 24 ++++++++++++++---------- code/read.lisp | 30 ++++++++++++++++++------------ test/regressions.lisp | 17 +++++++++++++++++ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/code/cache.lisp b/code/cache.lisp index 82ba83a..4f38e15 100644 --- a/code/cache.lisp +++ b/code/cache.lisp @@ -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) @@ -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) diff --git a/code/read.lisp b/code/read.lisp index ec9daaf..39a420f 100644 --- a/code/read.lisp +++ b/code/read.lisp @@ -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))) @@ -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))))))) diff --git a/test/regressions.lisp b/test/regressions.lisp index 9c7598f..defd6b9 100644 --- a/test/regressions.lisp +++ b/test/regressions.lisp @@ -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)))