bug#58706: 29.0.50; [noverlay] `get-pos-property' misses overlay start

bug#58706: 29.0.50; [noverlay] `get-pos-property' misses overlay start

[Matt Armstrong]

X-Debbugs-CC: Stefan Monnier <monnier@iro.umontreal.ca>

The following test passes on mainline but doesn't on feature/noverlay.
On the noverlay branch `get-pos-property' returns nil.

(ert-deftest test-get-pos-property-overlay-beg ()
  "Test `get-pos-property' at the beginning of an overlay."
  (with-temp-buffer
    (insert (make-string 10 ?x))
    (let ((overlay (make-overlay 5 7)))
      (overlay-put overlay 'forty-two 42))
    (should (equal 42 (get-pos-property 5 'forty-two)))))

bug#58706: 29.0.50; [noverlay] `get-pos-property' misses overlay start

[Matt Armstrong]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

Matt Armstrong <matt@rfc20.org> writes:

> X-Debbugs-CC: Stefan Monnier <monnier@iro.umontreal.ca>
>
> The following test passes on mainline but doesn't on feature/noverlay.
> On the noverlay branch `get-pos-property' returns nil.
>
> (ert-deftest test-get-pos-property-overlay-beg ()
>   "Test `get-pos-property' at the beginning of an overlay."
>   (with-temp-buffer
>     (insert (make-string 10 ?x))
>     (let ((overlay (make-overlay 5 7)))
>       (overlay-put overlay 'forty-two 42))
>     (should (equal 42 (get-pos-property 5 'forty-two)))))

Tests and a fix.  Also up on the scratch/matta/for_stefan branch of
https://git.sr.ht/~matta/emacs


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Add-get-pos-property-tests-covering-bug-58706.patch --]
[-- Type: text/x-diff, Size: 2809 bytes --]

From 555bc1f8b3ed8b02fb5acb013ed24073b0666585 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Sat, 22 Oct 2022 20:46:30 -0700
Subject: [PATCH 2/3] Add `get-pos-property' tests covering bug#58706

* test/src/buffer-tests.el (get-pos-property-overlay-beg): New test.
(get-pos-property-overlay-empty-rear-advance): ditto.
(get-pos-property-overlay-past-rear-advance): ditto.
(get-pos-property-overlay-at-narrowed-end): ditto.
---
 test/src/buffer-tests.el | 43 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el
index c6d176cc171..e0207325241 100644
--- a/test/src/buffer-tests.el
+++ b/test/src/buffer-tests.el
@@ -1108,6 +1108,49 @@ test-delete-all-overlay-1
     (should-not (delete-all-overlays))))
 
 
+;; +==========================================================================+
+;; | get-pos-property
+;; +==========================================================================+
+
+(ert-deftest get-pos-property-overlay-beg ()
+  "Test `get-pos-property' at the beginning of an overlay.
+Regression test for bug#58706."
+  (with-temp-buffer
+    (insert (make-string 10000 ?x))
+    (let ((overlay (make-overlay 9999 10001)))
+      (overlay-put overlay 'forty-two 42))
+    (should (equal 42 (get-pos-property 9999 'forty-two)))))
+
+(ert-deftest get-pos-property-overlay-empty-rear-advance ()
+  "Test `get-pos-property' at the end of an empty rear-advance overlay.
+Regression test for bug#58706."
+  (with-temp-buffer
+    (insert (make-string 10000 ?x))
+    (let ((overlay (make-overlay 9999 9999 nil nil t)))
+      (overlay-put overlay 'forty-two 42))
+    (should (equal 42 (get-pos-property 9999 'forty-two)))))
+
+(ert-deftest get-pos-property-overlay-past-rear-advance ()
+  "Test `get-pos-property' past the end of an empty rear-advance overlay.
+Regression test for bug#58706."
+  (with-temp-buffer
+    (insert (make-string 10000 ?x))
+    (let ((overlay (make-overlay 9998 9998 nil nil t)))
+      (overlay-put overlay 'forty-two 42))
+    (should (equal nil (get-pos-property 9999 'forty-two)))))
+
+(ert-deftest get-pos-property-overlay-at-narrowed-end ()
+  "Test `get-pos-property' at the end of a narrowed region.
+Regression test for bug#58706."
+  (with-temp-buffer
+    (insert (make-string 11000 ?x))
+    (narrow-to-region 9998 10000)
+    (let ((overlay (make-overlay 10000 10000 nil t nil)))
+      (overlay-put overlay 'forty-two 42))
+    (should (equal nil (get-pos-property 9999 'forty-two)))))
+
+;; FIXME: add more `get-pos-property' tests
+
 ;; +==========================================================================+
 ;; | get-char-property(-and-overlay)
 ;; +==========================================================================+
-- 
2.35.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0003-Fix-get-pos-property-for-the-new-overlay-implementat.patch --]
[-- Type: text/x-diff, Size: 2331 bytes --]

From b78be2bf7a9ac8b71d25529d5736373f51852c36 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Sat, 22 Oct 2022 20:48:10 -0700
Subject: [PATCH 3/3] Fix `get-pos-property' for the new overlay
 implementation.

Some of the trickier edge cases weren't handled properly.  See
bug#58706.

* src/editfns.c (overlays_around): Extend the search range past POS by
one to fetch overlays beginning at POS.  Fetch empty overlays, as they
may be extended by an insertion and thus be relevant to
`get-pos-property'.  Make a note that the function now, unfortunately,
may return out of range overlays.
(Fget_pos_property): Deal with 'overlays_around' returning out of
range overlays.
---
 src/editfns.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/editfns.c b/src/editfns.c
index 00380175859..c892c02927d 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -265,12 +265,20 @@ DEFUN ("mark-marker", Fmark_marker, Smark_marker, 0, 0, 0,
 \f
 /* Find all the overlays in the current buffer that touch position POS.
    Return the number found, and store them in a vector in VEC
-   of length LEN.  */
+   of length LEN.
+
+   Note: this can return overlays that do not touch POS.  The caller
+   should filter these out. */
 
 static ptrdiff_t
 overlays_around (ptrdiff_t pos, Lisp_Object *vec, ptrdiff_t len)
 {
-  return overlays_in (pos - 1, pos, false, &vec, &len, false, false, NULL);
+  /* Find all potentially rear-advance overlays at (POS - 1).  Find
+     all overlays at POS, so end at (POS + 1).  Find even empty
+     overlays, which due to the way 'overlays-in' works implies that
+     we might also fetch empty overlays starting at (POS + 1).  */
+  return overlays_in (pos - 1, pos + 1, false, &vec, &len,
+		      true, false, NULL);
 }
 
 DEFUN ("get-pos-property", Fget_pos_property, Sget_pos_property, 2, 3, 0,
@@ -333,7 +341,9 @@ DEFUN ("get-pos-property", Fget_pos_property, Sget_pos_property, 2, 3, 0,
 	      if ((OVERLAY_START (ol) == posn
 		   && OVERLAY_FRONT_ADVANCE_P (ol))
 		  || (OVERLAY_END (ol) == posn
-		      && ! OVERLAY_REAR_ADVANCE_P (ol)))
+		      && ! OVERLAY_REAR_ADVANCE_P (ol))
+		  || OVERLAY_START (ol) > posn
+		  || OVERLAY_END (ol) < posn)
 		; /* The overlay will not cover a char inserted at point.  */
 	      else
 		{
-- 
2.35.1

bug#58706: 29.0.50; [noverlay] `get-pos-property' misses overlay start

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Tests and a fix.  Also up on the scratch/matta/for_stefan branch of
> https://git.sr.ht/~matta/emacs

Thanks, merged,


        Stefan

bug#58706: 29.0.50; [noverlay] `get-pos-property' misses overlay start

[Stefan Kangas]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Tests and a fix.  Also up on the scratch/matta/for_stefan branch of
>> https://git.sr.ht/~matta/emacs
>
> Thanks, merged,

Bug was left open, so I'm closing it now.