Stefan Monnier wrote:
>> You originally objected to my patch slowing down Emacs.  So I looked for
>> optimization opportunities to ensure that my patch paid for its performance
>> costs.  I was successful, and not only did I offset the costs, I even
>> produced a net improvement in speed, and provided benchmarks to prove it.
>
> I'm sorry, but I consider this a form of lying.  It makes it sound like
> "the new var-hook functionality actually speeds things up", even though
> it's not this new functionality but some unrelated (tho bundled) change
> which does it (and which could be applied independently).

I didn't say the varhook functionality speeds things up. I explicitly said it has performance costs, and I found optimization opportunities to _pay for_ those costs.

And again, I didn't say I eliminated the costs; I said I _offset_ the costs, and did better than just break even.

I was completely honest about what I did, and why I did it. And you accuse me of lying?

> The issue is not patch size per se, but just keeping the patch focused
> on its core purpose.

The constant-hooked combination is an optimization, not part of the patch's core purpose, yet you told me to include it anyway. Varhook works just fine without that optimization (and the patch is simpler without it), as shown by the varhook-single.patch I submitted on Feb 5th.

The patch could be applied to trunk without that optimization, then the optimization applied later, along with all the rest, without affecting the functionality of Emacs in general or of the varhook feature in particular.

But I'm leaving it in the patch, just because you asked me to put it there. I don't want to bother to take it back out now, and IIUC, you still want me to leave it in.

> "unnecessary" and "pathological" are judgments which are actually hard
> to make for such a generic hooking functionality where we don't (want
> to) know what the applications will be.
>
> As a general design principle Emacs doesn't really try to prevent you
> from shooting yourself in the foot.

Fine, but adding the capability of converting setq, etc into makunbound has the cost of breaking correctness (because hooking a symbol would change the behavior of (setq foo void-sentinel)), not just the cost of enabling user errors. Remember, this capability isn't something I'm removing from Emacs; it's just an additional capability that varhook could add. The benefit isn't worth the cost.


Updated patch attached. Changes, as you requested:
setq, etc return the attempt value instead of the override value.
One bug fixed.
Another bug intentionally added.
The void sentinel value is un-marked as special, and the constant is renamed.
Function names changed yet again. Now they all start with ⌜symbol-hook⌝.
All mention of «advice» banished from the documentation.
The documentation is more explicit about the cases in which each environment is affected.
As much as possible without making the documentation incomprehensible, it now conflates symbols with global variables.
The field name in Lisp_Symbol changed back to the misleading name, just to avoid touching a few lines of code in the patch.
Magic constants added back in to the source code, just to avoid touching a few lines of code in the patch.
All optimizations removed, except for the particular one (combining constant and hooked) that you told me to include, which happens to be the most invasive one, yet with no more benefit than the others.

This version of the patch slows down Emacs.

IIUC, I've made all the changes you requested.