From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Noam Postavsky <npostavs@users.sourceforge.net>
Cc: John Wiegley <jwiegley@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
emacs-devel@gnu.org
Subject: Re: Lisp watchpoints
Date: Sat, 28 Nov 2015 23:44:19 -0500 [thread overview]
Message-ID: <jwvsi3pqw8j.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <CAM-tV-_mKG_o=ixndnXxFdzp4fgV-fLmxgZSJPsi6u1BiLNJfw@mail.gmail.com> (Noam Postavsky's message of "Sat, 28 Nov 2015 21:04:25 -0500")
> @@ -629,9 +634,18 @@ DEFUN ("makunbound", Fmakunbound, Smakunbound, 1, 1, 0,
> (register Lisp_Object symbol)
> {
> CHECK_SYMBOL (symbol);
> - if (SYMBOL_CONSTANT_P (symbol))
> - xsignal1 (Qsetting_constant, symbol);
> - Fset (symbol, Qunbound);
> + switch (XSYMBOL (symbol)->trapped_write)
> + {
> + case SYMBOL_NOWRITE:
> + xsignal1 (Qsetting_constant, symbol);
> +
> + case SYMBOL_TRAPPED_WRITE:
> + notify_variable_watchers (symbol, Qnil, Qunbind, Qnil);
> + /* fallthrough */
> + case SYMBOL_UNTRAPPED_WRITE:
> + default:
> + Fset (symbol, Qunbound);
> + }
> return symbol;
> }
Hmm... `unbind' is the term usually used when exiting a let binding
rather than when setting a var to Qunbound. So I think it'd be better
here to use Qmakunbound to avoid confusion.
> + set_symbol_trapped_write (symbol, SYMBOL_UNTRAPPED_WRITE); /* avoid recursion */
Please capitalize and punctaute your comments. Also, try not to exceed
80 columns.
> + ptrdiff_t count = SPECPDL_INDEX ();
> + record_unwind_protect (&reset_symbol_trapped_write, symbol);
^^^
I think this "&" is unneeded.
> + while (!NILP (watchers))
> + {
> + Lisp_Object watcher = XCAR (watchers);
This will lead to a segmentation fault after something like (put <var>
'watchers [1]). While very unlikely, it's very easy to avoid this
problem: just use CONSP instead of !NILP.
> + else if (FUNCTIONP (watcher))
> + CALLN (Ffuncall, watcher, operation, where, symbol, newval);
I don't think you need to test "FUNCTIONP (watcher)".
And you don't need Ffuncall: what you wrote is similar to writing
(funcall #'funcall watcher operation where symbol newval)
> - if (sym->constant)
> + if (sym->trapped_write == SYMBOL_NOWRITE)
> error ("Symbol %s may not be frame-local", SDATA (SYMBOL_NAME (variable)));
Actually, trapped writes won't work reliably for frame-local vars
(because you can set them with things like `put' or `setcar' rather than
let/setq/...), so better disallow combining the two (frame-local vars
are obsolete anyway and should be removed "soonish", so it's fine not
to provide support for new features for them).
Stefan
next prev parent reply other threads:[~2015-11-29 4:44 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-14 18:14 Lisp watchpoints (Was: [Emacs-diffs] master 19e09cf: Ensure redisplay after evaluation) Noam Postavsky
2015-11-14 22:29 ` Lisp watchpoints Stefan Monnier
2015-11-22 20:13 ` Noam Postavsky
2015-11-22 20:41 ` Eli Zaretskii
2015-11-22 21:25 ` Stefan Monnier
2015-11-23 3:32 ` Eli Zaretskii
2015-11-23 2:31 ` Noam Postavsky
2015-11-29 2:04 ` Noam Postavsky
2015-11-29 4:44 ` Stefan Monnier [this message]
2015-11-29 5:12 ` Noam Postavsky
2015-11-29 11:28 ` Andreas Schwab
2015-11-29 14:12 ` Noam Postavsky
2015-11-29 15:37 ` Stefan Monnier
2015-11-29 16:10 ` Eli Zaretskii
2015-11-30 0:57 ` Noam Postavsky
2015-11-30 1:14 ` Stefan Monnier
2015-11-30 15:49 ` Eli Zaretskii
2015-11-29 11:28 ` Andreas Schwab
2015-11-29 15:39 ` Stefan Monnier
2015-11-29 15:45 ` Eli Zaretskii
2015-11-29 16:01 ` Noam Postavsky
2015-11-29 16:04 ` Noam Postavsky
2015-11-29 17:13 ` Eli Zaretskii
2015-11-29 22:24 ` Stefan Monnier
2015-11-29 17:11 ` Eli Zaretskii
2015-11-29 16:06 ` Stefan Monnier
2015-11-29 16:43 ` Eli Zaretskii
2015-11-29 19:40 ` Noam Postavsky
2015-11-29 19:54 ` Eli Zaretskii
2015-11-29 20:35 ` Noam Postavsky
2015-11-29 20:45 ` Eli Zaretskii
2015-11-29 21:33 ` Noam Postavsky
2015-11-29 20:00 ` Andreas Schwab
2015-11-29 20:19 ` Eli Zaretskii
-- strict thread matches above, loose matches on Subject: below --
2016-12-03 14:51 Kaushal Modi
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=jwvsi3pqw8j.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
--cc=jwiegley@gmail.com \
--cc=npostavs@users.sourceforge.net \
/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.