From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Kelly Dean Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] (Updated) Run hook when variable is set Date: Sat, 14 Feb 2015 22:19:45 +0000 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1423952495 7593 80.91.229.3 (14 Feb 2015 22:21:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 14 Feb 2015 22:21:35 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Feb 14 23:21:22 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YMl5V-0007Mm-3Z for ged-emacs-devel@m.gmane.org; Sat, 14 Feb 2015 23:21:21 +0100 Original-Received: from localhost ([::1]:33026 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMl5U-0003a0-0e for ged-emacs-devel@m.gmane.org; Sat, 14 Feb 2015 17:21:20 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMl5N-0003Zk-Uf for emacs-devel@gnu.org; Sat, 14 Feb 2015 17:21:16 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMl5G-0000az-9i for emacs-devel@gnu.org; Sat, 14 Feb 2015 17:21:13 -0500 Original-Received: from relay5-d.mail.gandi.net ([2001:4b98:c:538::197]:52842) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMl5G-0000ah-1M for emacs-devel@gnu.org; Sat, 14 Feb 2015 17:21:06 -0500 Original-Received: from mfilter22-d.gandi.net (mfilter22-d.gandi.net [217.70.178.150]) by relay5-d.mail.gandi.net (Postfix) with ESMTP id C9E2241C04B; Sat, 14 Feb 2015 23:21:04 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mfilter22-d.gandi.net Original-Received: from relay5-d.mail.gandi.net ([217.70.183.197]) by mfilter22-d.gandi.net (mfilter22-d.gandi.net [10.0.15.180]) (amavisd-new, port 10024) with ESMTP id 0AbfNWfmKrig; Sat, 14 Feb 2015 23:21:03 +0100 (CET) X-Originating-IP: 66.220.3.179 Original-Received: from localhost (gm179.geneticmail.com [66.220.3.179]) (Authenticated sender: kelly@prtime.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id BCC5741C061; Sat, 14 Feb 2015 23:20:57 +0100 (CET) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4b98:c:538::197 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:183074 Archived-At: Stefan Monnier wrote: >> The docstring for symbol-setter-function says lexical binding is requi= red >> for the hook function, > > I don't think that's true, BTW. I agree it's recommended (I recommend > using lexical-binding for all new code), but other than that it should > work just fine with dynamically scoped code. It was to enable the standard function to always just return newval, even= if it's Qunbound. Works for lexical binding (the interpreter doesn't bar= f), but not for dynamic. But that doesn't matter anymore, since you said = I need to use a different way of handling Qunbound. >> The docstring already says to use :before if you just need to watch >> variables, > > It shouldn't either. This issue of which kind of advice to use is not > specific to this variable, so there's no need to put it there. It's an unusual function. The effect of overriding the return value, and = interaction with =C2=ABlet=C2=BB, and how to deal with the case of makunb= ound, all need to be documented somewhere, and for completeness, it makes= sense for the documentation to also cover the case of not overriding the= return value. If not in the docstring, then maybe in the manual? > Actually I don't see these as two independent elements combined > into one. I see it as a single value with 3 levels: > - any write you want > - write vetted by Elisp code > - no write at all If there's just :before advice (e.g. a profiler), then the write is not v= etted. > We could even drop the last setting and implement it in Elisp via > symbol-setter-function, except that it's currently used for variables > where we'd rather not give Elisp the opportunity to allow writes. Yeah, changing the value of t or nil would be quite fun. ;-) >> +DEFUN ("void-p", Fvoid_p, Svoid_p, 1, UNEVALLED, 0, [snip] > I doubt this works on lexical-vars in compiled code. Why do we need it= ? It was to enable checking for Qunbound for lexical newval and oldval in s= ymbol-setter-function. I've removed it. > So far we've been extra careful to try and make sure Qunbound doesn't > escape into Elisp world. I don't think we want to reconsider this > design choice, since adding this Qunbound as a "normal" value in Elisp > will just create the need for another special "really unbound" value. Qunbound already escapes: you just have to use makunbound instead of =C2=AB= set=C2=BB to set it, and boundp instead of symbol-value to read it. Lexic= al variables are second-class because they can't be set to all the values= that dynamic variables can be; the purpose of my =C2=ABvoid=C2=BB functi= on was to fix that. Of course, you'll say makunbound doesn't set a value; it unbinds the vari= able. But that's not true! For example, consider this: (defvar foo 0) (defvar bar 1) (let ((foo 2)) bar) =E2=86=92 1 (let ((foo 2)) (makunbound 'foo) foo) =E2=86=92 Lisp error: (void-variabl= e foo) foo =E2=86=92 0 In the dynamic environment created by the first =C2=ABlet=C2=BB, foo is b= ound, but bar is not bound. Therefore an attempt to read the not-bound ba= r within that environment will look in outer environments until it finds = bar bound in one of them. An error would occur if bar weren't bound in an= y of those outer environments, not even the global one. In the environment created by the second =C2=ABlet=C2=BB, foo is first bo= und to 2. If it were unbound, then an attempt to read it would return 0, = for the same reason that an attempt to read bar in the first environment = returns 1. So what does makunbound do? If it really unbound foo, then the= subsequent attempt to read foo would return 0, because foo would be not = bound in the dynamic environment. But in fact the attempt to read foo ret= urns Qunbound, which was the value set by makunbound, and the interpreter= barfs because you didn't use the special function boundp to read that sp= ecial value. IOW, Qunbound escapes into the Lisp world. Then when the =C2=ABlet=C2=BB exits, the dynamic environment is destroyed= , which actually _does_ unbind foo (from Qunbound). So now, the attempt t= o read foo returns 0, because global foo is no longer shadowed by the dyn= amic binding of foo to the value Qunbound. I agree that setting lexicals to Qunbound is absurd. =C2=ABvoid=C2=BB was= just to enables lexicals and dynamics to be set to the same values. My p= oint is that letting Qunbound escape into the Lisp world in the first pla= ce is also absurd. (CL does it too.) Makunbound should actually make a va= riable be unbound, not bind it to some special value that triggers an err= or (and lies about the cause) when you try to read it. Because it makes n= o sense to actually unbind a dynamic or lexical variable except by destru= ction of the environment in which the variable occurs, this means makunbo= und should apply only to global variables. Because global variables don't shadow anything, binding them to Qunbound = is the same as unbinding them so far as Lisp code is concerned, so this i= mplementation detail doesn't escape. And Lisp doesn't let you bind lexica= ls to Qunbound, so they don't have a problem either. But it _does_ let yo= u bind dynamic variables to Qunbound (and Elisp lets you bind buffer-loca= ls to Qunbound too), which is a problem because they shadow variables in = outer environments. I know it's officially been a feature for a long time= . But a bug by any other name is still a bug. I've removed =C2=ABvoid=C2=BB. I recommend making makunbound stop working= on non-globals too. > I see some alternatives: [snip] > - OLDVAL is either a list of one element containing the old value, or > nil (when that old value is Qunbound). Then run_varhook must cons. That'll generate a lot of garbage if you use = it for profiling, or for debugging in a way where you don't just pause to= inspect every hooked variable. Is that ok? > - Use a special value to represent "unbound", e.g. a special keyword li= ke > `::value--unbound::'. Then varhook would be defective. If the hook function returns a special k= eyword to represent Qunbound (to avoid blocking makunbound), then just ho= oking unavoidably changes the behavior of Emacs (because it's impossible = to set a hooked variable to that keyword), which is the wrong thing to do= . Of course, telling the hook function that a variable is bound to Qunbound= without having to cons is easy (could just pass an additional argument, = t or nil). The problem is needing to _return_ Qunbound. If run_varhook wi= ll cons, then the hook function can just return nil to represent Qunbound= , or do (setcar newval ...) and then return newval to set the variable to= another value without needing yet another cons. Or to avoid needing to cons in run_varhook, it could specbind something l= ike Qsymbol_newval_void (to t if the new value is Qunbound), so the hook = function could setq symbol-newval-void to t or nil to indicate whether to= set the hooked variable to Qunbound. This way, run_varhook could pass th= e newval argument directly (as it currently does), without wrapping it, e= xcept pass nil as a dummy value if the new value is Qunbound. This is wha= t I prefer, but if you don't want me to do it this way, then I'll wrap ne= wval. > Oh, and I think we can keep the name `constant' rather than use the > verbose `constant_or_hooked'. It's not perfect, but at least it reduce= s > the patch's size and the number of lines that are too long. Would =E2=8C=9Cconst_hooked=E2=8C=9D be ok? It's only 4 extra characters.= Using =E2=8C=9Cconstant=E2=8C=9D for something that doesn't just mean =C2= =ABconstant=C2=BB is as gross as =E2=8C=9Cset-default=E2=8C=9D for =C2=AB= set-global=C2=BB. As noted above, hooked doesn't imply vetted, so hooked = doesn't mean sort-of-constant. Even if it did, =E2=8C=9Cconstant=E2=8C=9D= is a misleading term for something that can be sort-of-constant; =E2=8C=9C= constantness=E2=8C=9D would be better, since the term allows for the poss= ibility of a range of values. If it's too long, =E2=8C=9Cconstness=E2=8C=9D= is only 1 extra character. The patch is big, but it'll just be a one-time change to the source code.= After that, nobody reading the code will care what the patch was, and ev= erybody will be stuck with the misleading name if it isn't changed.