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: Permission requested to merge branch comment-cache into master. Date: Mon, 14 Mar 2016 11:58:26 -0400 Message-ID: References: <83mvq4gni4.fsf@gnu.org> <20160312100843.GA2572@acm.fritz.box> <83a8m4gd7p.fsf@gnu.org> <20160312115021.GB2572@acm.fritz.box> <837fh7ho78.fsf@gnu.org> <20160312231713.GD10781@acm.fritz.box> <20160313152017.GD1871@acm.fritz.box> <20160314132806.GE1894@acm.fritz.box> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1457971180 32399 80.91.229.3 (14 Mar 2016 15:59:40 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 14 Mar 2016 15:59:40 +0000 (UTC) Cc: emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Mar 14 16:59:27 2016 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 1afUty-0005aJ-9I for ged-emacs-devel@m.gmane.org; Mon, 14 Mar 2016 16:59:26 +0100 Original-Received: from localhost ([::1]:41912 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afUtx-00010p-Ku for ged-emacs-devel@m.gmane.org; Mon, 14 Mar 2016 11:59:25 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51053) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afUtA-0008KH-B8 for emacs-devel@gnu.org; Mon, 14 Mar 2016 11:58:37 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afUt6-0005vk-4q for emacs-devel@gnu.org; Mon, 14 Mar 2016 11:58:36 -0400 Original-Received: from pruche.dit.umontreal.ca ([132.204.246.22]:53679) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afUt5-0005vg-V5 for emacs-devel@gnu.org; Mon, 14 Mar 2016 11:58:32 -0400 Original-Received: from pastel.home (lechon.iro.umontreal.ca [132.204.27.242]) by pruche.dit.umontreal.ca (8.14.7/8.14.1) with ESMTP id u2EFwQGD026020; Mon, 14 Mar 2016 11:58:27 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 63CF86012E; Mon, 14 Mar 2016 11:58:26 -0400 (EDT) In-Reply-To: <20160314132806.GE1894@acm.fritz.box> (Alan Mackenzie's message of "Mon, 14 Mar 2016 13:28:06 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.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 RV5610=0 X-NAI-Spam-Version: 2.3.0.9418 : core <5610> : inlines <4493> : streams <1602849> : uri <2165728> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.22 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:201704 Archived-At: >> The only case where it calls to user function is via syntax-propertize. >> In the case of back_comment this should never happen (because we're >> moving backward, so syntax-propertize has already been called on >> positions further down and hence isn't needed at this position), .... > That's not true. If you have just moved forward in the buffer to a part > where syntax-properties haven't yet been applied, and do a > (forward-comment -1), syntax-propertize would get called for that buffer > position. This should generally not be the case, no, because syntax-propertize will have been called during the SETUP_SYNTAX_TABLE call earlier. There might be corner cases where it is called, tho, admittedly. Some of them may be bugs, obviously. >> but in case it were to happen it could only be because it's necessary >> *for correctness*. > It might be necessary to apply those properties for correctness, but > applying them is NOT the job of a primitive. Depends if you want the primitive to work correctly. >> So, again: when my patch to back_comment calls syntax-ppss, it will not >> call syntax-propertize-function. > It will. The only case I know of where this can happen is if parse-sexp-lookup-properties is nil (which is unusual since as soon as syntax-propertize-function is called, it sets parse-sexp-lookup-properties to t). If needed we could try and plug this corner case, tho it hasn't been a problem so far. > I have an idea. How about rewriting the cache mechanism in C, caching > the value MUCH more frequently (say, every 128 bytes) and using a data > structure suitable for that amount of data? Be my guest. It's always been part of the possible future I was imagining for syntax-ppss, so I'm definitely not opposed to it. That won't remove the need to call syntax-propertize, of course. And I'm not sure it's worth the trouble since I haven't seen any indication that the performance of the current implementation of syntax-ppss is a problem. > That's the job of high level code, most likely part of a major mode. > The primitive should _respect_ the syntax-table properties which are > there, but has no business applying them itself. Indeed the way they > are applied and when they are applied is entirely for the major mode to > decide. That's indeed what CC-mode does. All other modes just use syntax-propertize which takes care of that for them. I have no reason to believe that CC-mode couldn't also use syntax-propertize (tho it would take a lot of work to restructure CC-mode accordingly, IIUC). But it doesn't really matter: for various reasons, CC-mode does it differently, and that's OK. It means that in your case, syntax-ppss will not call syntax-propertize-function, so the problems you're so worried about won't affect you. > OK, sorry about the invective, but take that away and what I said is > accurate. You have construed the problem as much more limited in scope > than I have, and you would leave back_comment largely unchanged. That's right, my patch leaves back_comment largely unchanged, and I do think you're making mountains of mole-hills. >> Now, I'm far from opposed to changing the behavior so as to treat the >> last /* '"'" */ as a comment in the narrowed region. I think this >> choice doesn't really matter much, except for a few specific >> circumstances, where the caller needs to provide more info in order to >> clarify which behavior she wants. > Here's where we differ. I think primitives should be rigorous and > always work, even in awkward corner cases. Then you'd better start looking at the whole narrowing issue much more seriously, because we don't handle it consistently at all: syntax-ppss is just one of many culprits. > And that is wholly uncalled for. My solution is rock-solid and > bullet-proof, barring any remaining bugs in it due to fact that it's new > code. Of course it's not. According to your own description, it'll break when syntax-table is changed; it will break when category properties are changed; it will break when the buffer's text is changed while inhibit-modification-hooks is non-nil. Additionally to that it will break code which relies on `forward-comment' (and all other primitives that depend on parse-sexp-ignore-comments, such as scan-sexp, forward-sexp, up-list, ...) working *within* the narrowed region without widening. I don't fault your code for that, mind you. It's just that this is Elisp we're talking about, not Coq. The foundations on which you build this have fuzzy&complex semantics. Stefan