From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] (Updated) Run hook when variable is set Date: Fri, 06 Feb 2015 09:42:18 -0500 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 1423233753 25050 80.91.229.3 (6 Feb 2015 14:42:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 6 Feb 2015 14:42:33 +0000 (UTC) Cc: emacs-devel@gnu.org To: Kelly Dean Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Feb 06 15:42:32 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 1YJk76-0001iW-Bp for ged-emacs-devel@m.gmane.org; Fri, 06 Feb 2015 15:42:32 +0100 Original-Received: from localhost ([::1]:48711 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJk75-00077a-Ku for ged-emacs-devel@m.gmane.org; Fri, 06 Feb 2015 09:42:31 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJk70-00077U-TO for emacs-devel@gnu.org; Fri, 06 Feb 2015 09:42:28 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJk6v-00076c-Kf for emacs-devel@gnu.org; Fri, 06 Feb 2015 09:42:26 -0500 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:43146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJk6v-00076W-FV for emacs-devel@gnu.org; Fri, 06 Feb 2015 09:42:21 -0500 Original-Received: from pastel.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id t16EgJEE032664; Fri, 6 Feb 2015 09:42:19 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id C3C50FAE; Fri, 6 Feb 2015 09:42:18 -0500 (EST) In-Reply-To: (Kelly Dean's message of "Fri, 06 Feb 2015 05:34:41 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV5209=0 X-NAI-Spam-Version: 2.3.0.9393 : core <5209> : inlines <2042> : streams <1385745> : uri <1848089> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 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:182529 Archived-At: > 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 =C2=ABconstant=C2=BB, =E2=80=9FI= f the value is 3, > then the var can be changed, but only by `defconst'.=E2=80=9D 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 =E2=8C=9Cvarhook=E2=8C=9D 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 actua= lly > 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 =C2=ABhooked=C2=BB is left as a separate bit, it will = cost > practically nothing extra compared to making it be part of =C2=ABconstant= =C2=BB (and > the latter would increase complexity and require code duplication). Is th= ere > 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 =E2=8C=9Csymbol-watch-set=E2=8C=9D, = =E2=8C=9Csymbol-watch-unset=E2=8C=9D > and =E2=8C=9Csymbol-watch-p=E2=8C=9D would be more accurate, Fine by me. > because those functions set/check the =C2=ABhooked=C2=BB 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 =E2=8C=9Cvariable-watch-function=E2=8C=9D 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