bug#70187: [PATCH] Fix + ert for 'makunbound' and "local if set" notification bugs (6 of 9)

bug#70187: [PATCH] Fix + ert for 'makunbound' and "local if set" notification bugs (6 of 9)

[Robert Burks]

[-- Attachment #1.1: Type: text/plain, Size: 3573 bytes --]

(6 of 9)

Bug#00005 (Buffer local makunbounds lack 'where' in notification)
** Bug recreations are at the end

It wasn't until the last line of the test for the previous bug until I
realized
these existed. I have included a patch to fix this bug and the prior along
with
ert.  I have also included a patch that corrects an incorrect testing
assumption.
This is the only existing should form that I have found that was in error.
(This subject widely lacked testing previously.)

When a "local if set" variable or variable made local to a specific buffer
is
unbound in a buffer the notification should contain the location.

This bug could only be solved after the others as prior to my changes
notification was handled in aggregate at the top of set functions.
Handling these
bugs was dependent on notification being handled based on the redirect
path. This
could actually be applied after "bug#00001", I just worked on this last (It
took
a bit of gdb tracing to root out) and I didn't want to go back through and
renumber all my writing. (During proofreading way later I laughed because
there
ended up being far more after this.)

I have also included ert for previously untested error cases in
set_internal,
set_default_internal, and defvaralias.

Patch 0016: Fix for bugs 00004 and 00005 (four(4) places require bug#
update)

Patch 0017: Ert for bug#00004 (four(4) places require bug# update)

Patch 0018: Corrected one 'should' form to include buffer name

Patch 0019: Ert for bug#00005 (four(4) places require bug# update)

Patch 0020: Added ert for functions that lacked testing code coverage for
            basic input errors.

Bug
Recreation------------------------------------------------------------------

Watched variable becoming local in some buffer then being unbound.
---------------------------------------------------------------------------------------
(defvar test 5)
test

(defvar results nil)
results

(add-variable-watcher 'test (lambda (&rest args)
                              (push args results)))
nil

(with-temp-buffer
  (make-local-variable 'test)
  (set 'test 100)
  (makunbound 'test))
test
5

(set 'test 100)
100

results
((test 100 set nil) (test nil makunbound nil) (test 100 set #<killed
buffer>))
;; test is still bound in this buffer but 'where' is 'nil', it was unbound
in the temp buffer.
;; The final set shows nil as it should for a global.

(set 'results nil)
nil

(with-temp-buffer
  (make-local-variable 'test)
  (set 'test 100)
  (makunbound 'test)
  (set 'test 200))
200

test
100

(set 'test 300)

results
((test 300 set nil) (test 200 set #<killed buffer>) (test nil makunbound
nil) (test 100 set #<killed buffer>))
;; a watcher needs to know where this makunbound happened.

---------------------------------------------------------------------------------------
A "local if set" become unbound in some buffer,
---------------------------------------------------------------------------------------
(defvar-local test 5)
test

(defvar results nil)
results

(add-variable-watcher 'test (lambda (&rest args)
                              (push args results)))
nil

(with-temp-buffer
  (set 'test 100)
  (makunbound 'test))
test

(set 'test 100)
100

results
((test 100 set #<buffer *scratch*>) (test nil makunbound nil) (test 100 set
#<killed buffer>))

(set 'results nil)
nil

(with-temp-buffer
  (set 'test 100)
  (makunbound 'test)
  (set 'test 200))
200

test
100

(set 'test 300)
300

results
((test 300 set #<buffer *scratch*>) (test 200 set #<killed buffer>) (test
nil makunbound nil) (test 100 set #<killed buffer>))

[-- Attachment #1.2: Type: text/html, Size: 4261 bytes --]

[-- Attachment #2: 0017-Added-ert-for-no-blv-watcher-unlet-notification-BUG-.patch --]
[-- Type: application/x-patch, Size: 2936 bytes --]

[-- Attachment #3: 0019-Add-ert-for-missing-where-makunbound-watcher-bug-BUG.patch --]
[-- Type: application/x-patch, Size: 1940 bytes --]

[-- Attachment #4: 0016-Fixes-for-two-variable-watcher-bugs-BUG-00004-BUG-00.patch --]
[-- Type: application/x-patch, Size: 3910 bytes --]

[-- Attachment #5: 0020-Added-ert-for-previously-untested-for-error-cases.patch --]
[-- Type: application/x-patch, Size: 2927 bytes --]

[-- Attachment #6: 0018-Fix-testing-assumptions-corrected-with-BUG-00005.patch --]
[-- Type: application/x-patch, Size: 1126 bytes --]

bug#70187: [PATCH] Fix + ert for 'makunbound' and "local if set" notification bugs (6 of 9)

[Eli Zaretskii]

> From: Robert Burks <rburksdev@gmail.com>
> Date: Thu, 4 Apr 2024 04:46:26 -0400
> 
> (6 of 9)
> 
> Bug#00005 (Buffer local makunbounds lack 'where' in notification)
> ** Bug recreations are at the end           
> 
> It wasn't until the last line of the test for the previous bug until I realized
> these existed. I have included a patch to fix this bug and the prior along with
> ert.  I have also included a patch that corrects an incorrect testing assumption.
> This is the only existing should form that I have found that was in error.
> (This subject widely lacked testing previously.)
> 
> When a "local if set" variable or variable made local to a specific buffer is
> unbound in a buffer the notification should contain the location.
> 
> This bug could only be solved after the others as prior to my changes
> notification was handled in aggregate at the top of set functions.  Handling these
> bugs was dependent on notification being handled based on the redirect path. This
> could actually be applied after "bug#00001", I just worked on this last (It took
> a bit of gdb tracing to root out) and I didn't want to go back through and
> renumber all my writing. (During proofreading way later I laughed because there
> ended up being far more after this.)
> 
> I have also included ert for previously untested error cases in set_internal,
> set_default_internal, and defvaralias. 
> 
> Patch 0016: Fix for bugs 00004 and 00005 (four(4) places require bug# update)
> 
> Patch 0017: Ert for bug#00004 (four(4) places require bug# update)
> 
> Patch 0018: Corrected one 'should' form to include buffer name
> 
> Patch 0019: Ert for bug#00005 (four(4) places require bug# update)
> 
> Patch 0020: Added ert for functions that lacked testing code coverage for
>             basic input errors.
> 
> Bug Recreation------------------------------------------------------------------
> 
> Watched variable becoming local in some buffer then being unbound.
> ---------------------------------------------------------------------------------------
> (defvar test 5)
> test
> 
> (defvar results nil)
> results
> 
> (add-variable-watcher 'test (lambda (&rest args)
>                               (push args results)))
> nil
> 
> (with-temp-buffer
>   (make-local-variable 'test)
>   (set 'test 100)
>   (makunbound 'test))
> test
> 5
> 
> (set 'test 100)
> 100
> 
> results
> ((test 100 set nil) (test nil makunbound nil) (test 100 set #<killed buffer>))
> ;; test is still bound in this buffer but 'where' is 'nil', it was unbound in the temp buffer. 
> ;; The final set shows nil as it should for a global.
> 
> (set 'results nil)
> nil
> 
> (with-temp-buffer
>   (make-local-variable 'test)
>   (set 'test 100)
>   (makunbound 'test)
>   (set 'test 200))
> 200
> 
> test
> 100
> 
> (set 'test 300)
> 
> results
> ((test 300 set nil) (test 200 set #<killed buffer>) (test nil makunbound nil) (test 100 set #<killed buffer>))
> ;; a watcher needs to know where this makunbound happened.
> 
> ---------------------------------------------------------------------------------------
> A "local if set" become unbound in some buffer,
> ---------------------------------------------------------------------------------------
> (defvar-local test 5)
> test
> 
> (defvar results nil)
> results
> 
> (add-variable-watcher 'test (lambda (&rest args)
>                               (push args results)))
> nil
> 
> (with-temp-buffer
>   (set 'test 100)
>   (makunbound 'test))
> test
> 
> (set 'test 100)
> 100
> 
> results
> ((test 100 set #<buffer *scratch*>) (test nil makunbound nil) (test 100 set #<killed buffer>))
> 
> (set 'results nil)
> nil
> 
> (with-temp-buffer
>   (set 'test 100)
>   (makunbound 'test)
>   (set 'test 200))
> 200
> 
> test
> 100
> 
> (set 'test 300)
> 300
> 
> results
> ((test 300 set #<buffer *scratch*>) (test 200 set #<killed buffer>) (test nil makunbound nil) (test 100 set
> #<killed buffer>))

Stefan, any comments on the issues and the patches?  Note that the
patches were updated later in bug#70189.

Thanks.