From: Eli Zaretskii <eliz@gnu.org>
To: Robert Burks <rburksdev@gmail.com>,
Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 70188@debbugs.gnu.org
Subject: bug#70188: [PATCH] Fix + ert for notification sending 'set unbound' when unbinding (7 of 9)
Date: Sat, 06 Apr 2024 10:42:16 +0300 [thread overview]
Message-ID: <86jzlb3pk7.fsf@gnu.org> (raw)
In-Reply-To: <CAHvcHq6FitSbJrqE45NRPWbxFOg8CWVQNBv_hPTSGohDnY8Btg@mail.gmail.com> (message from Robert Burks on Thu, 4 Apr 2024 04:46:47 -0400)
> From: Robert Burks <rburksdev@gmail.com>
> Date: Thu, 4 Apr 2024 04:46:47 -0400
>
> (7 of 9) (Resistance is futile)
>
> Bug#00006 (The mysterious unlet to 'unbound')
> ** Bug recreations are at the end
>
> Once again the path from do_one_unbind leads to rare cases in
> set_default_internal. I have included a fix and ert.
>
> The case is do_one_unbind calls set_default_internal (foo , Qunbound, SET_INTERNAL_UNBIND)
> -- We reach here with Qunbound as a value only when unletting if when 'letting'
> the buffer saw the void default/global value.
> -- make_local_foo called for the first time within a let on a variable that is globally void.
> -- make_local_foo was called in some other buffer and a let takes place in a buffer that
> still sees the default as void. (let_shadows_default)
> -- The 'local_if_set' shadows default case cannot reach here with 'unbound' as a value,
> even with something like this:
> (defvar foo)
> (make-variable-buffer-local 'foo)
> This will establish a default of 'nil'.
> -- I don't think there are any other ways set_default_internal is being called
> with Qunbound as value on localized variables.
> -- My included test is the only test in data-tests.el(maybe all testing) that
> hits the above conditions.
>
> -- Currently it stores Qunbound to the cdr of blv->defcell and returns. This seems correct.
> -- I have only tested plain values. Forward symbols like DEFVAR_LISP and DEFVAR_BOOL
> (not DEFVAR_PER_BUFFER) could reach here with 'Qunbound' as 'new value' if they start
> their life unbound. If makunbound is used on one they get turned to plain value
> by set_internal. But if they are set unbound in 'C' who knows.
> Maybe we need to check for void and do blv->fwd.fwdptr = NULL? Will need testing...
>
> First example is 'make_local_foo' happened in some other buffer so the 'let' in this
> buffer now shadows the default.
>
> Second example is 'make_local_foo' happened in a 'let' making it 'unlet' to default.
>
> There still may exist other edge cases, I am working to uncover those. Every bug
> example I have sent has been fixed by the patches included in these seven emails.
>
> Additionally, the work I have submitted does not change the way Emacs handles the
> numerous scenarios around variable setting; it merely makes watcher notification
> reflect the already defined behavior.
>
> Patch 0021: Fix for this bug (one(1) place requires bug# update)
>
> Patch 0022: Make it so functions pass 'Qunbound' to 'notify' and do conversion
> to 'Qnil' there. This allows easier tracking in gdb, it was hard
> to isolate prior. Distinction may also be needed in the future by
> watchers.
>
> Patch 0023: ert for this bug (four(4) places require bug# update)
>
> Bug Recreation------------------------------------------------------------------
>
> This path can only be reached through the special path from do_one_unbind that
> leads to set_default_internal.
> --------------------------------------------------------------------------------
> (defvar test)
> test
>
> (defvar results nil)
> results
>
> (add-variable-watcher 'test (lambda (&rest args)
> (push args results)))
> nil
>
> (with-temp-buffer
> (make-local-variable 'test)
> (let ((test 99))
> (set 'test 66)))
> 66
>
> (let ((test 99))
> (set 'test 66))
> 66
>
> results
> ((test unbound set nil) (test 66 set nil) (test 99 let nil) (test nil unlet #<killed buffer>) (test 66 set #<killed
> buffer>) (test 99 let #<killed buffer>))
> ;; notice it says 'unbound' vs 'nil', all other functions that notify use 'nil'
> ;; for the unbound case. 'test' is still unbound globally. It is returning a
> ;; potential legitimate symbol 'unbound' because it is sending the 'C' string
> ;; definition for Qunbound;
> ;; Also, this should say 'unlet' (This was fixed with the bug#00004/5 solution)
> --------------------------------------------------------------------------------
> Extra broke case
> --------------------------------------------------------------------------------
> (defvar test)
> test
>
> (defvar results nil)
> results
>
> (add-variable-watcher 'test (lambda (&rest args)
> (push args results)))
> nil
>
> (let ((test 99))
> (make-local-variable 'test)
> (set 'test 66))
> 66
>
> test
> 66
>
> results
> ((test unbound set nil) (test 66 set #<buffer *scratch*>) (test 99 let nil))
> ;; should be (test nil unlet nil) as it is now 66 in *scratch* but void globally
>
> Not a Bugged Case****************************************************************
> Notice this case works. One of the rare cases already tested for.
> --------------------------------------------------------------------------------
> (defvar test)
> test
>
> (defvar results nil)
> results
>
> (add-variable-watcher 'test (lambda (&rest args)
> (push args results)))
> nil
>
> (make-local-variable 'test)
> test
>
> (let ((test 99))
> (set 'test 66))
> 66
>
> results
> ((test nil unlet #<buffer *scratch*>) (test 66 set #<buffer *scratch*>) (test 99 let #<buffer *scratch*>))
This changes how the watchers are called, so it needs suitable changes
in NEWS in and in the ELisp manual.
Stefan, any comments?
prev parent reply other threads:[~2024-04-06 7:42 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-04 8:46 bug#70188: [PATCH] Fix + ert for notification sending 'set unbound' when unbinding (7 of 9) Robert Burks
2024-04-06 7:42 ` Eli Zaretskii [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86jzlb3pk7.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=70188@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
--cc=rburksdev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.