Re: [PATCH] (Updated) Run hook when variable is set

[Stefan Monnier]

> Oh, I see. Yes, that would avoid the extra conditional branch in
> specbind

Exactly.

> (but not in unbind_to).

Actually, we could also avoid this extra cost if we don't push a normal
specbind record on the pdl, but instead we'd push a record_unwind that
will call the hook function again on the way back.
I haven't looked at whether that could be done without too much
gymnastics, tho.  So maybe the extra check in unbind_to is OK.

> But avoiding the conditional branch that way for the set_internal function
> (or set_internal_1 in my patch) would require duplicating the entire
> function, in both the source code and the compiled code.

I don't understand why.

> The definition of Lisp_Symbol says about «constant», ‟If the value is 3,
> then the var can be changed, but only by `defconst'.”  I assumed that comment
> was accurate.

Oh, sorry, that must have been a messed up commit of mine (I've had such
a tentative change in my local tree, but I ended up not installing it,
since defconst vars are modified much too often).

> The ⌜varhook⌝ symbol is the hook (an ordinary hook) that's run when hooked
> symbols are set.

Yes, I saw it afterwards.  But the name was too generic, making it sound
like a local var or local function.

>> The hook should be called *to perform the assignment*, rather than
>> calling it after performing the assignment ourselves.
> I'm confused. You mean if a symbol is hooked, then when you attempt to set
> it (using the usual setq, etc), the hook should be run _instead_ of actually
> setting the symbol?

That's right.  This way the hook can see the value before the assignment
(very useful for watchpoint-style debugging) and can decide to block the
assignment altogether (e.g. let's say you want to prevent assignment of
values of the wrong type to a particular variable; or you want to
implement the "defconst makes the variable read-only", ...).

> I don't see the reason for doing this.

Because it's more general.  The real question is: why do it otherwise?

I happen to have an answer for that question as well: because, if
the assignment calls the hook, how does the hook perform the assignment?
(I.e. we need some way to perform the assignment without triggering the
hook)

Ideally this is done by having two layers: a top-layer that triggers the
hook(s) and a bottom layer that performs a "raw" access.  That's how
I did it for defalias-vs-fset, but for setq we don't have 2 layers yet,
so either we need to add a new layer (and since we want the hook to
trigger on plain old `setq' it means the new layer has to be the "raw"
assignment).

> to be consistent with the surrounding code because I thought I added both.)

Indeed, we're not consistent in this respect, but I don't think
consistency is that important here, and if I get to choose I prefer the
prototypes with variable names than without.

> So I think even if «hooked» is left as a separate bit, it will cost
> practically nothing extra compared to making it be part of «constant» (and
> the latter would increase complexity and require code duplication). Is there
> someone on the mailing list who understands branch prediction well and
> can comment?

Branch prediction should work well, indeed, but you still have extra
costs (e.g. because of the density of branches, for example, and CPUs
tend to be unable to predict more than one branch per cycle, because of
the need to extract the bit, ...).

So the slowdown might not be terrible, but remember this is a very core
part of Elisp execution (slightly less so with lexical-scoping, but
still), so the impact could be measurable.  This downside has to be
weighted against the fact that noone is using this feature right now,
and there hasn't been many requests for it (I've been toying with the
idea for many years, but never bumped into a strong enough need for it
to get me coding).

> Ok.  However (just a nitpick), using ⌜symbol-watch-set⌝, ⌜symbol-watch-unset⌝
> and ⌜symbol-watch-p⌝ would be more accurate,

Fine by me.

> because those functions set/check the «hooked» bit not just for
> a particular variable, but for all non-lexical variables of the
> specified symbol (i.e. all occurrences of the symbol in all
> non-lexical run-time environments: global, buffer-local, and
> dynamic).

[ The difference between symbol and variables in Elisp's dynamic binding
  is quite messy indeed.  `symbol-watch' has the downside that it might
  make it sound like `fset' and `set-symbol-plist' would also
  be hooked.  ]

> But ⌜variable-watch-function⌝ is not misleading, since it doesn't suggest
> that it applies to just one particular variable.  It applies to all
> hooked variables.

But please use `symbol-watch-function' for consistency.


        Stefan